From 1b31e2f189b9833c706e7ccf3306308df3463493 Mon Sep 17 00:00:00 2001 From: PJ Date: Fri, 28 Aug 2026 10:19:52 +0530 Subject: [PATCH] Let a code span carry the emphasis marks, so bold code opens at all --- src/editor/Editor.tsx | 16 +++++++- src/editor/extensions.test.ts | 38 +++++++++++++++++++ src/markdown/adversarial5.test.ts | 14 ++++--- .../corpus/adversarial/nested-marks.md | 5 ++- src/markdown/index.ts | 26 ++++++++++++- src/markdown/parse.ts | 7 +++- src/markdown/roundtrip.test.ts | 24 +++++++++++- src/model/schema.test.ts | 7 +++- src/model/schema.ts | 10 ++++- 9 files changed, 131 insertions(+), 16 deletions(-) diff --git a/src/editor/Editor.tsx b/src/editor/Editor.tsx index d76c02d..9f43da4 100644 --- a/src/editor/Editor.tsx +++ b/src/editor/Editor.tsx @@ -20,6 +20,7 @@ import type { Editor } from "@tiptap/react"; import { EditorState, TextSelection } from "@tiptap/pm/state"; import type { EditorProps as ProseMirrorProps, EditorView } from "@tiptap/pm/view"; import type { CalloutKind, HeadingLevel, MarkdownDocument } from "../model/doc"; +import { sourceDocument } from "../markdown"; import { marks as markSpecs } from "../model/schema"; import type { MarkName } from "../model/schema"; import { notify } from "../store/useToast"; @@ -535,7 +536,20 @@ export function DocumentEditor({ return EditorState.create({ doc, plugins: base.plugins }); } catch (error) { reportContentError(error); - return EditorState.create({ schema: base.schema, plugins: base.plugins }); + // Never an empty document. `check` answers for the whole tree, so one text node the schema + // will not hold used to blank the file on screen, and the file on screen is what the next + // keystroke saves: the debounce would then write those few characters over the bytes on + // disk. The file's own source, in one raw block, is a document that always passes and that + // a save writes back verbatim, so the worst case is a document shown as source rather than + // a document destroyed. + try { + const doc = ed.schema.nodeFromJSON(sourceDocument(source).toJSON()); + doc.check(); + return EditorState.create({ doc, plugins: base.plugins }); + } catch (fallback) { + reportContentError(fallback); + return EditorState.create({ schema: base.schema, plugins: base.plugins }); + } } }; diff --git a/src/editor/extensions.test.ts b/src/editor/extensions.test.ts index d8ddca3..7697ae2 100644 --- a/src/editor/extensions.test.ts +++ b/src/editor/extensions.test.ts @@ -8,6 +8,7 @@ import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; import { createEditorExtensions } from "./extensions"; import { schema as contract } from "../model/schema"; import { parseMarkdown, serializeMarkdown } from "../markdown"; +import { corpus } from "../markdown/corpus/load"; const extensions = () => createEditorExtensions({ documentPath: () => "/notes/a.md", onError: () => {} }); @@ -146,6 +147,43 @@ describe("the generated schema", () => { // the serializer has to read node names rather than node types. This is that, asserted. expect(serializeMarkdown(parsed, rebound)).toBe(serializeMarkdown(parsed, parsed.doc)); }); + + // The gate that was missing. Every other sweep reads a document the bridge built, and a document + // the bridge built is not necessarily one the editor will accept: `check()` is what src/editor/ + // Editor.tsx asks before it installs the state, and a document that fails it is not partly + // refused, it is replaced by an empty one and the whole file goes blank on screen. Nothing here + // called it on a parsed document, so a paragraph holding "**`x`**" opened as nothing at all. + it("holds every corpus file, checked the way the editor checks it", () => { + const found: string[] = []; + for (const file of corpus()) { + const parsed = parseMarkdown(file.source, `/${file.name}`); + try { + built.nodeFromJSON(parsed.doc.toJSON()).check(); + } catch (error) { + found.push(`${file.name}: ${String(error)}`); + } + } + expect(found).toEqual([]); + }); + + it("holds a code span carrying every mark that can be wrapped around one", () => { + for (const source of ["**`x`**\n", "_`x`_\n", "~~`x`~~\n", "[`x`](./y.md)\n"]) { + const parsed = parseMarkdown(source, "/notes/a.md"); + const rebound = built.nodeFromJSON(parsed.doc.toJSON()); + expect(() => rebound.check(), source).not.toThrow(); + expect(serializeMarkdown(parsed, rebound), source).toBe(source); + } + + // All of them on one span. The spelling moves, because marks are a set and MARK_ORDER decides + // the nesting once for every document, so what is asserted is that it settles there and stays. + const source = "**~~[`x`](./y.md)~~**\n"; + const parsed = parseMarkdown(source, "/notes/a.md"); + expect(() => built.nodeFromJSON(parsed.doc.toJSON()).check()).not.toThrow(); + const once = serializeMarkdown(parsed, parsed.doc); + expect(once).toBe("[~~**`x`**~~](./y.md)\n"); + const again = parseMarkdown(once, "/notes/a.md"); + expect(serializeMarkdown(again, again.doc)).toBe(once); + }); }); describe("the editor built from them", () => { diff --git a/src/markdown/adversarial5.test.ts b/src/markdown/adversarial5.test.ts index 2ad2e26..e96a4c2 100644 --- a/src/markdown/adversarial5.test.ts +++ b/src/markdown/adversarial5.test.ts @@ -327,13 +327,17 @@ const CONTAINERS: Array<[string, (inline: ProseMirrorNode[]) => ProseMirrorNode] const MARKS = ["link", "strikethrough", "strong", "em", "code"]; -/** Every pair the schema actually permits: code excludes the formatting marks, and is a leaf. */ +/** + * Every pair the schema actually permits: code is a leaf, so it is only ever the inner one. + * + * The inner code pairs were skipped here while the code mark excluded the formatting group, which + * is how `**`x`**` reached a release: the sweep could not build the one document that broke. + */ function pairs(): Array<[string, string]> { const out: Array<[string, string]> = []; for (const outer of MARKS) { for (const inner of MARKS) { if (outer === inner || outer === "code") continue; - if (inner === "code" && outer !== "link") continue; out.push([outer, inner]); } } @@ -343,8 +347,8 @@ function pairs(): Array<[string, string]> { describe("still fixed: every mark nested inside every other, in every block", () => { // The fourth pass found the delete handler's missing whitespace guard and said why three passes // had missed it: the sweeps carried strikethrough and link as sibling snippets and never nested - // them. This is that gap closed. Thirteen ordered pairs, five spanning shapes, three boundary - // paddings and eleven containers, which is 2145 documents, each built from the schema, written, + // them. This is that gap closed. Sixteen ordered pairs, five spanning shapes, three boundary + // paddings and eleven containers, which is 2640 documents, each built from the schema, written, // read back and then saved ten more times. it("keeps every mark over every span, in every container, without moving or growing", () => { @@ -380,7 +384,7 @@ describe("still fixed: every mark nested inside every other, in every block", () } } - expect(checked, "the sweep has to actually be the size it claims").toBe(2145); + expect(checked, "the sweep has to actually be the size it claims").toBe(2640); expect(found).toEqual([]); }, 120000); diff --git a/src/markdown/corpus/adversarial/nested-marks.md b/src/markdown/corpus/adversarial/nested-marks.md index 80520f0..545fff9 100644 --- a/src/markdown/corpus/adversarial/nested-marks.md +++ b/src/markdown/corpus/adversarial/nested-marks.md @@ -9,8 +9,9 @@ Strong inside a strikethrough, and a strikethrough inside strong: Emphasis inside a link, and a link inside emphasis: [a\_b\_c](./x.md) and _a_[_b_](./y.md)_c_. -Code inside a link, which is the only pair code takes: -[`code span`](./z.md) and [a`b`c](./w.md). +Code inside a link, inside each emphasis mark, and inside all of them at once: +[`code span`](./z.md) and [a`b`c](./w.md) and **`bold code`** and _`em code`_ and +~~`struck code`~~ and **~~[`the lot`](./v.md)~~**. All four at once, over a boundary space: **q**~~** r **~~**s** diff --git a/src/markdown/index.ts b/src/markdown/index.ts index e8e9551..5303148 100644 --- a/src/markdown/index.ts +++ b/src/markdown/index.ts @@ -29,7 +29,8 @@ import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; import { schema } from "../model/schema"; import type { MarkdownDocument } from "../model/doc"; -import { normaliseSource, splitFrontmatter, withFrontmatter } from "./frontmatter"; +import { rawNode } from "../model/doc"; +import { BOM, normaliseSource, splitFrontmatter, withFrontmatter } from "./frontmatter"; import { parseToMdast } from "./handlers"; import { buildDoc } from "./parse"; import { serializeBody } from "./serialize"; @@ -79,6 +80,29 @@ export function serializeMarkdown(document: MarkdownDocument, doc: ProseMirrorNo return withFrontmatter(document.frontmatter, serializeBody(doc, document.frontmatter === null)); } +/** + * The whole body as one raw block: the file, shown as its own source. + * + * The last resort for a document the editor will not hold. The bridge refuses a construct by making + * a raw block of it, which is the same answer at a smaller scale, and a raw block the user has not + * typed in is written back byte for byte, so a file opened this way and saved is the file that was + * read. The alternative that was here, an empty document, is the one outcome the module's fourth + * invariant exists to forbid: the file looks empty on screen and the first keystroke saves it that + * way over the bytes on disk. + */ +export function sourceDocument(document: MarkdownDocument): ProseMirrorNode { + const { text } = normaliseSource(document.source); + // The prefix is the frontmatter as it will be written back, and it carries the byte order mark + // when there is one. `text` has already had that mark taken off, so putting it into the slice + // offset would cut the body's first character off with it. + const head = document.frontmatter ?? ""; + const prefix = head.startsWith(BOM) ? head.slice(BOM.length) : head; + const body = text.slice(prefix.length); + const doc = schema.nodes.doc.createAndFill(null, body ? [rawNode(body)] : []); + if (!doc) throw new Error("the source could not be held as a raw block"); + return doc; +} + /** * A .txt file, which the tree marks editable and the editor opens alongside markdown. * diff --git a/src/markdown/parse.ts b/src/markdown/parse.ts index 275b861..bb988b7 100644 --- a/src/markdown/parse.ts +++ b/src/markdown/parse.ts @@ -446,7 +446,10 @@ function inlineFrom(nodes: PhrasingContent[], marks: readonly Mark[]): ProseMirr break; } case "inlineCode": { - if (node.value) out.push(schema.text(node.value, [...marks, m.code.create()])); + // addToSet for the same reason the wrappers use it: a spread is sorted by `Mark.setFrom` + // but is not put through the marks' own exclusion rules, so it can build a set the schema + // does not allow and that `doc.check()` refuses, which blanks the whole document. + if (node.value) out.push(schema.text(node.value, m.code.create().addToSet(marks))); break; } case "link": { @@ -456,7 +459,7 @@ function inlineFrom(nodes: PhrasingContent[], marks: readonly Mark[]): ProseMirr // dropping a destination quietly is the one thing this bridge is here not to do. if (marks.some((mark) => mark.type === m.link)) return null; const mark = m.link.create({ href: node.url, title: node.title ?? null }); - const inner = inlineFrom(node.children, [...marks, mark]); + const inner = inlineFrom(node.children, mark.addToSet(marks)); // A mark needs something to sit on, so a link with no text at all, `[](./x.md)`, has // nowhere to live in the document and would be dropped along with its destination. if (!inner || inner.length === 0) return null; diff --git a/src/markdown/roundtrip.test.ts b/src/markdown/roundtrip.test.ts index 6a62c89..6297886 100644 --- a/src/markdown/roundtrip.test.ts +++ b/src/markdown/roundtrip.test.ts @@ -5,7 +5,18 @@ import { describe, expect, it } from "vitest"; import type { Node as ProseMirrorNode } from "@tiptap/pm/model"; import { schema } from "../model/schema"; import { corpus } from "./corpus/load"; -import { parseMarkdown, serializeMarkdown } from "./index"; +import { parseMarkdown, serializeMarkdown, sourceDocument } from "./index"; +import { BOM, normaliseSource } from "./frontmatter"; + +/** + * The source as a save of it writes it back: CRLF collapsed, the byte order mark still there, and + * the one line ending the house style ends a file with. Everything else is the file's own bytes. + */ +function settled(source: string): string { + const { text, bom } = normaliseSource(source); + const body = text === "" || text.endsWith("\n") ? text : `${text}\n`; + return bom ? BOM + body : body; +} const files = corpus(); @@ -187,6 +198,17 @@ describe("opening a document", () => { } }); + // The fallback the editor installs when a document is one it cannot hold. It has to be worth + // more than the empty document it replaced, which means the bytes have to survive a save. + it("can be shown as its own source, and written back as the bytes that were read", () => { + for (const file of files) { + const document = parseMarkdown(file.source, `/corpus/${file.name}`); + const shown = sourceDocument(document); + expect(shown.childCount, file.name).toBeLessThanOrEqual(1); + expect(serializeMarkdown(document, shown), file.name).toBe(settled(file.source)); + } + }); + it("reaches nothing outside itself", () => { const sources = import.meta.glob("./*.ts", { query: "?raw", import: "default", eager: true }) as Record; for (const [name, text] of Object.entries(sources)) { diff --git a/src/model/schema.test.ts b/src/model/schema.test.ts index 6047aa8..b5aeee6 100644 --- a/src/model/schema.test.ts +++ b/src/model/schema.test.ts @@ -240,10 +240,13 @@ describe("inline", () => { expect(link.toJSON()).toEqual({ type: "link", attrs: { href: "./a.md", title: "A" } }); }); - it("lets a code span sit inside a link but not inside emphasis", () => { + it("lets a code span sit inside a link and inside emphasis", () => { const code = m.code.create(); expect(code.isInSet(m.link.create({ href: "./a.md" }).addToSet([code]))).toBeTruthy(); - expect(m.strong.create().addToSet([code])).toEqual([code]); + for (const outer of [m.strong, m.em, m.strikethrough]) { + const set = outer.create().addToSet([code]); + expect([outer.name, set.map((mark) => mark.type.name)]).toEqual([outer.name, [outer.name, "code"]]); + } }); it("treats math and images as inline atoms", () => { diff --git a/src/model/schema.ts b/src/model/schema.ts index cec45d0..0f31905 100644 --- a/src/model/schema.ts +++ b/src/model/schema.ts @@ -433,10 +433,16 @@ export const marks: { [name in MarkName]: MarkSpec } = { toDOM: () => ["s", 0], }, - // A code span is literal, so it excludes the emphasis marks, but not link: [`x`](y) is valid. + // A code span's own content is literal, so nothing can be emphasised inside one, but a code span + // can itself be emphasised or linked: `**\`x\`**` and `[\`x\`](y)` are both ordinary markdown and + // both render on GitHub. Marks are a flat set here, so those two readings are the same set and + // the direction is decided once, by the serializer: src/markdown/serialize.ts puts code innermost + // and writes the emphasis around it. Excluding the formatting group instead, which this mark did + // until a document holding `**\`x\`**` could not be opened at all, is not a narrower rule but a + // wrong one: the bridge kept building the set the file described and every such document failed + // `doc.check()` on the way into the editor. code: { code: true, - excludes: "formatting code", parseDOM: [{ tag: "code" }], toDOM: () => ["code", 0], },