diff --git a/packages/prosemirror-markdown/src/lib/attached-comment.ts b/packages/prosemirror-markdown/src/lib/attached-comment.ts new file mode 100644 index 00000000..5df81d00 --- /dev/null +++ b/packages/prosemirror-markdown/src/lib/attached-comment.ts @@ -0,0 +1,88 @@ +/** + * Attached-comment convention (#293 canon). + * + * Some block-level attributes have no native markdown syntax (paragraph/heading + * `textAlign` — #9; image/media attrs — #4/#8). Rather than HTML-wrapping the + * whole block (the old `
` / `

` forms, which the maintainer + * had to patch repeatedly and which did not round-trip cleanly), we ATTACH a + * compact HTML comment at the END of the block's rendered line: + * + * Some paragraph text + * + * The comment is invisible in any markdown renderer and is dropped by the + * DOM/generateJSON import stage, so it can never leak into the document body. + * The importer intercepts it BEFORE that stage (see markdown-to-prosemirror's + * applyAttachedComments) and re-applies the encoded attributes to the node. + * + * This module holds the two PURE, reusable primitives of the convention so the + * serializer, the parser, and future decisions (#4 image, #8 media) share ONE + * implementation: + * - `attachedCommentFor(name, json)` — build the comment string. + * - `parseAttachedComment(data)` — parse a comment node's data back. + */ + +/** + * A parsed attached comment: the leading `name` token and the decoded JSON + * object payload (empty object when the comment carried no JSON body). + */ +export interface AttachedComment { + name: string; + attrs: Record; +} + +/** + * Grammar of an attached comment's DATA (the text between ``): + * a leading name token (`attrs`, `img`, …) optionally followed by whitespace + * and a single JSON object. The name deliberately does NOT allow `:` so the + * file-level envelope comments (`docmost:meta` / `docmost:comments`) never match + * and stay inert here. + */ +const ATTACHED_COMMENT_RE = /^\s*([A-Za-z][\w-]*)(?:\s+(\{[\s\S]*\}))?\s*$/; + +/** + * Build an attached HTML comment `` for `json`. + * + * The JSON is emitted compactly (no spaces) via `JSON.stringify`. A string value + * may legitimately contain two consecutive hyphens `--`, which would prematurely + * close the HTML comment (`-->`). We defuse that WITHOUT changing the decoded + * value: each hyphen of every `--` pair is rewritten as the JSON unicode escape + * `-`, so `JSON.parse` on the reading side restores the exact original + * hyphens. `--` can only occur inside a JSON string (structural JSON never + * produces it), so a blanket replace over the stringified payload is safe. + */ +export function attachedCommentFor(name: string, json: object): string { + const raw = JSON.stringify(json); + // Escape every hyphen that is part of a `--` pair. Scanning left-to-right and + // replacing each `--` handles odd runs too (`---` -> two escapes + one bare + // `-`, still `---` after JSON.parse). + const safe = raw.replace(/--/g, "\\u002d\\u002d"); + return ``; +} + +/** + * Parse the DATA of a comment node into `{ name, attrs }`, or `null` when it is + * not a well-formed attached comment. + * + * Fail-open by design (maintainer spec): a comment whose name token is missing, + * whose JSON body is malformed, or whose body is not a plain object returns + * `null` so the caller ignores it and keeps default attributes. Unknown keys in + * a valid object are preserved here and filtered by the caller. + */ +export function parseAttachedComment(data: string): AttachedComment | null { + const m = ATTACHED_COMMENT_RE.exec(data); + if (!m) return null; + const name = m[1]; + if (m[2] === undefined) { + // Name-only comment (no JSON body): a valid attached marker with no attrs. + return { name, attrs: {} }; + } + try { + const parsed = JSON.parse(m[2]); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + return { name, attrs: parsed as Record }; + } + return null; // fail-open: payload is not a plain object + } catch { + return null; // fail-open: malformed JSON -> ignore the comment + } +} diff --git a/packages/prosemirror-markdown/src/lib/index.ts b/packages/prosemirror-markdown/src/lib/index.ts index f02713e5..f37c47b5 100644 --- a/packages/prosemirror-markdown/src/lib/index.ts +++ b/packages/prosemirror-markdown/src/lib/index.ts @@ -24,6 +24,15 @@ export { markdownToProseMirror } from "./markdown-to-prosemirror.js"; // schema the converter targets. export { docmostExtensions } from "./docmost-schema.js"; +// Attached-comment convention (#293 canon #9/#4/#8): the reusable primitives +// the serializer/parser use to encode attrs that have no native markdown syntax +// as trailing `` comments. +export { + attachedCommentFor, + parseAttachedComment, +} from "./attached-comment.js"; +export type { AttachedComment } from "./attached-comment.js"; + export { canonicalizeContent, docsCanonicallyEqual, diff --git a/packages/prosemirror-markdown/src/lib/markdown-converter.ts b/packages/prosemirror-markdown/src/lib/markdown-converter.ts index 013a54f3..2a2a290d 100644 --- a/packages/prosemirror-markdown/src/lib/markdown-converter.ts +++ b/packages/prosemirror-markdown/src/lib/markdown-converter.ts @@ -1,4 +1,5 @@ import { encodeHtmlEmbedSource } from "./docmost-schema.js"; +import { attachedCommentFor } from "./attached-comment.js"; /** * Hard cap on processNode recursion depth (see the depth guard below). @@ -138,38 +139,45 @@ export function convertProseMirrorToMarkdown(content: any): string { case "doc": return nodeContent.map(processNode).join("\n\n"); - case "paragraph": + case "paragraph": { const text = nodeContent.map(processNode).join(""); const align = node.attrs?.textAlign; - if (align && align !== "left") { - // Emit alignment as a styled `

` (review #10). The old - // `

` had NO matching import parse rule — the div was - // unwrapped and alignment lost on every round trip. A styled `

` - // round-trips: the paragraph parse rule (tag:"p") matches and the - // textAlign global-attribute parseHTML (docmost-schema) reads the style. - return `

${text}

`; + // Non-default alignment round-trips as an ATTACHED HTML comment at the + // END of the block line (#293 canon #9): + // `some text ` + // This replaces the old `

` wrapper (review #10, + // itself a replacement for the never-parsed `

`). The schema's + // textAlign default is `null`; "left" is the visual default the editor + // renders identically to null — neither emits a marker (no churn). The + // importer's applyAttachedComments step reads the comment back before the + // DOM stage drops it. We only attach when there is text to attach to: a + // lone comment on a blank line has no block to bind to on re-import. + if (align && align !== "left" && text) { + return `${text} ${attachedCommentFor("attrs", { textAlign: align })}`; } return text || ""; + } - case "heading": + case "heading": { const level = node.attrs?.level || 1; const headingText = nodeContent.map(processNode).join(""); + const headingLine = "#".repeat(level) + " " + headingText; const headingAlign = node.attrs?.textAlign; - if (headingAlign && headingAlign !== "left") { - // Emit alignment as a styled `` so it round-trips losslessly, - // symmetric to the paragraph case above (review F5/A1). The bare - // `## text` markdown form carries NO alignment, so an aligned heading - // would silently drop textAlign on export. A styled `` re-parses: - // the heading parse rule (tag:"h1".."h6") matches and the textAlign - // global-attribute parseHTML (docmost-schema) reads the style back, - // preserving BOTH level and textAlign. escapeAttr keeps the align - // value injection-safe, exactly like the paragraph arm. - return `${headingText}`; + // A non-default heading alignment attaches the same trailing comment + // (#293 canon #9), keeping the readable `## text` markdown form: + // `## Title ` + // Bare `## text` carries no alignment, so without this an aligned heading + // would silently drop textAlign on export. Replaces the old + // `` HTML form. "left"/null stay bare (no churn). + // Require headingText so an empty aligned heading stays bare `##` rather + // than emitting a comment with no visible element to attach to (matches + // the paragraph guard's `text` check — a lone comment has no block to + // bind on re-import). + if (headingAlign && headingAlign !== "left" && headingText) { + return `${headingLine} ${attachedCommentFor("attrs", { textAlign: headingAlign })}`; } - // No alignment (or the default "left"): keep the plain `## text` - // markdown form — HTML-ifying an unaligned heading would be needless - // churn, exactly as the paragraph case keeps plain text when unaligned. - return "#".repeat(level) + " " + headingText; + return headingLine; + } case "text": let textContent = node.text || ""; diff --git a/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts b/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts index 4466edf4..73ed5eb1 100644 --- a/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts +++ b/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts @@ -11,6 +11,7 @@ import { generateJSON } from "@tiptap/html"; import { JSDOM } from "jsdom"; import { marked } from "marked"; import { docmostExtensions } from "./docmost-schema.js"; +import { parseAttachedComment } from "./attached-comment.js"; // Setup DOM environment for Tiptap HTML parsing in Node.js const dom = new JSDOM(""); @@ -315,6 +316,64 @@ function bridgeTaskLists(html: string): string { return document.body.innerHTML; } +/** + * Re-apply ATTACHED HTML comments (#293 canon) before the DOM/generateJSON + * stage drops them. + * + * The serializer appends attributes that have no native markdown syntax as a + * trailing `` comment on the block's line (see + * attached-comment.ts). `marked` keeps that comment as an HTML comment NODE + * inside the block element (`

text

`), but the next stage + * (parse5/jsdom via generateJSON) discards comment nodes, so the attributes + * would be lost. This pass runs on the post-`marked` HTML: for every attached + * comment it re-expresses the encoded attributes in a form the schema's + * parseHTML already understands, then removes the comment so it cannot leak. + * + * For #9 the only handled name is `attrs`, and the only handled key is + * `textAlign` on a paragraph/heading: it is written as an inline + * `text-align` style on the parent element, which the docmost-schema textAlign + * global attribute reads back. Fail-open everywhere: a malformed comment (null + * from parseAttachedComment), an unknown name, a comment whose parent is not a + * `

`/``, or an unknown/empty attr value is left inert (the comment is + * dropped by generateJSON anyway). Future decisions (#4/#8) extend the + * per-name handling here without changing the parse primitive. + */ +function applyAttachedComments(html: string): string { + // Cheap early-out: no comments at all -> nothing to intercept. + if (!html.includes("` comment (replacing the old +// `` HTML form). It re-parses back to a heading // carrying BOTH the level and the textAlign, so the round-trip is lossless; an // UNaligned heading still emits the bare `## text` markdown form (no churn). // =========================================================================== @@ -801,21 +801,22 @@ const alignedHeading = (level: number, align: string, ...inline: any[]) => ({ }); describe('heading.textAlign round-trip (A1)', () => { - it('an aligned heading exports as (not bare ##)', () => { + it('an aligned heading keeps "## text" and attaches a comment (#293 #9)', () => { expect(convertProseMirrorToMarkdown(doc(alignedHeading(2, 'center', text('Title'))))).toBe( - '

Title

', + '## Title ', ); }); it('survives export -> import -> export losslessly (level AND textAlign preserved)', async () => { const input = alignedHeading(2, 'center', text('Title')); const { md1, doc2, md2 } = await roundTrip(input); - // Export direction: a styled , injection-safe via escapeAttr. - expect(md1).toBe('

Title

'); + // Export direction: `## Title` plus the attached alignment comment (#293 #9). + expect(md1).toBe('## Title '); // Import direction: re-parses to a heading node with the level AND textAlign - // (the raw HTML block flows through marked -> generateJSON, where - // the heading parse rule matches and the textAlign global attr reads the - // style back). Byte-stable second export closes the loop. + // (marked keeps the comment inside the

; applyAttachedComments re-expresses + // it as an inline style before generateJSON, where the heading parse rule + // matches and the textAlign global attr reads it back). Byte-stable second + // export closes the loop. const h = doc2.content[0]; expect(h.type).toBe('heading'); expect(h.attrs.level).toBe(2); diff --git a/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts b/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts index f8ba6b76..0a4b69c0 100644 --- a/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts +++ b/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts @@ -129,20 +129,19 @@ describe('inline-mark matrix (underline/sub/sup/highlight±color/textStyle/comme }); }); -describe('paragraph.textAlign ->

', () => { - it('non-default alignment emits an HTML

', () => { - // #7 fix: a non-default paragraph alignment now round-trips. It is exported - // as an HTML `

` (the schema's paragraph - // parseHTML reads `style="text-align"` back onto `textAlign` on import), so - // the alignment survives instead of collapsing to bare text. (The old - // `

` form was NOT re-parsed onto the paragraph and was - // therefore lossy.) +describe('paragraph.textAlign -> attached comment (#293 #9)', () => { + it('non-default alignment emits a trailing comment', () => { + // #293 canon #9: a non-default paragraph alignment now round-trips as an + // ATTACHED HTML comment at the END of the block line instead of the old + // `

` wrapper (which the maintainer had to patch + // A14->A15->A16). The importer's applyAttachedComments step reads the comment + // back onto `textAlign` before the DOM stage drops it. expect(c({ type: 'paragraph', attrs: { textAlign: 'center' }, content: [text('x')] })).toBe( - '

x

', + 'x ', ); }); - it('textAlign "left" (the default) is NOT wrapped', () => { + it('textAlign "left" (the default) emits NO comment', () => { expect(c({ type: 'paragraph', attrs: { textAlign: 'left' }, content: [text('x')] })).toBe('x'); }); }); diff --git a/packages/prosemirror-markdown/test/textalign.test.ts b/packages/prosemirror-markdown/test/textalign.test.ts new file mode 100644 index 00000000..7c8dcb89 --- /dev/null +++ b/packages/prosemirror-markdown/test/textalign.test.ts @@ -0,0 +1,160 @@ +import { describe, expect, it } from 'vitest'; +// Import DIRECTLY from src (NOT the docmost-client barrel, which pulls in +// collaboration.ts and mutates global DOM at import time). +import { convertProseMirrorToMarkdown } from '../src/lib/markdown-converter.js'; +import { markdownToProseMirror } from '../src/lib/markdown-to-prosemirror.js'; +import { + attachedCommentFor, + parseAttachedComment, +} from '../src/lib/attached-comment.js'; + +// #293 canon decision #9: paragraph/heading `textAlign` serializes as an +// ATTACHED HTML comment at the END of the block line — +// `some text ` +// — replacing the old `
` / `

` wrappers, which +// did NOT round-trip cleanly (alignment was lost on the first stabilize pass). +// These tests are non-vacuous: they assert the EXACT emitted markdown (so they +// fail against any wrapper form) AND that the alignment survives a full +// PM -> MD -> PM round trip (which the old `

` never did). + +const doc = (...nodes: any[]) => ({ type: 'doc', content: nodes }); +const text = (t: string) => ({ type: 'text', text: t }); +const para = (align: string | null, ...inline: any[]) => ({ + type: 'paragraph', + attrs: align === null ? {} : { textAlign: align }, + content: inline, +}); +const heading = (level: number, align: string | null, ...inline: any[]) => ({ + type: 'heading', + attrs: align === null ? { level } : { level, textAlign: align }, + content: inline, +}); + +// Find the first paragraph/heading node in a generated doc (skips the doc root). +const firstBlock = (d: any) => d.content?.[0]; + +describe('attached-comment primitives (reusable for #9/#4/#8)', () => { + it('attachedCommentFor emits a compact ``', () => { + expect(attachedCommentFor('attrs', { textAlign: 'center' })).toBe( + '', + ); + }); + + it('attachedCommentFor escapes a `--` pair so it cannot close the comment early', () => { + // A string value containing `--` would otherwise inject `-->`. Each hyphen of + // the pair is emitted as the JSON unicode escape -; JSON.parse restores + // the original hyphens on the reading side. + const s = attachedCommentFor('img', { alt: 'a--b' }); + expect(s).toBe(''); + // No premature `--` inside the payload (between the `` closer), so the comment cannot terminate early. + expect(s.slice(4, -3)).not.toContain('--'); + // Round-trip through the parser primitive restores the exact value. + const inner = s.slice(''.length); + expect(parseAttachedComment(inner)).toEqual({ name: 'img', attrs: { alt: 'a--b' } }); + }); + + it('parseAttachedComment fails open on malformed JSON and non-objects', () => { + expect(parseAttachedComment('attrs {not json}')).toBeNull(); + expect(parseAttachedComment('attrs [1,2]')).toBeNull(); + expect(parseAttachedComment(' ')).toBeNull(); + // Name-only comment is a valid marker with empty attrs. + expect(parseAttachedComment('attrs')).toEqual({ name: 'attrs', attrs: {} }); + }); +}); + +describe('paragraph.textAlign serialization (#293 #9)', () => { + for (const align of ['center', 'right', 'justify']) { + it(`paragraph textAlign "${align}" -> trailing comment`, () => { + expect(convertProseMirrorToMarkdown(doc(para(align, text('hello'))))).toBe( + `hello `, + ); + }); + } + + it('default textAlign (null) emits NO comment', () => { + expect(convertProseMirrorToMarkdown(doc(para(null, text('hello'))))).toBe('hello'); + }); + + it('"left" (visual default) emits NO comment', () => { + expect(convertProseMirrorToMarkdown(doc(para('left', text('hello'))))).toBe('hello'); + }); +}); + +describe('heading.textAlign serialization (#293 #9)', () => { + it('heading keeps "## text" and attaches the alignment comment', () => { + expect(convertProseMirrorToMarkdown(doc(heading(2, 'center', text('Title'))))).toBe( + '## Title ', + ); + }); + + it('default heading emits the bare "## text" form', () => { + expect(convertProseMirrorToMarkdown(doc(heading(3, null, text('Plain'))))).toBe('### Plain'); + }); +}); + +describe('paragraph.textAlign round-trip PM -> MD -> PM (#293 #9)', () => { + for (const align of ['center', 'right', 'justify']) { + it(`preserves paragraph textAlign "${align}"`, async () => { + const md1 = convertProseMirrorToMarkdown(doc(para(align, text('hello')))); + expect(md1).toBe(`hello `); + const doc2 = await markdownToProseMirror(md1); + const block = firstBlock(doc2); + expect(block.type).toBe('paragraph'); + expect(block.attrs.textAlign).toBe(align); + // Text is intact (the trailing space before the comment is trimmed). + expect(block.content?.[0]?.text).toBe('hello'); + // Byte-stable second export closes the loop. + expect(convertProseMirrorToMarkdown(doc2)).toBe(md1); + }); + } + + it('default paragraph re-imports with textAlign null (no comment survives)', async () => { + const md1 = convertProseMirrorToMarkdown(doc(para(null, text('hello')))); + const doc2 = await markdownToProseMirror(md1); + const block = firstBlock(doc2); + expect(block.type).toBe('paragraph'); + expect(block.attrs.textAlign ?? null).toBeNull(); + }); +}); + +describe('heading.textAlign round-trip PM -> MD -> PM (#293 #9)', () => { + for (const [level, align] of [ + [2, 'center'], + [3, 'right'], + [1, 'justify'], + ] as [number, string][]) { + it(`preserves h${level} textAlign "${align}"`, async () => { + const md1 = convertProseMirrorToMarkdown(doc(heading(level, align, text('Head')))); + expect(md1).toBe(`${'#'.repeat(level)} Head `); + const doc2 = await markdownToProseMirror(md1); + const block = firstBlock(doc2); + expect(block.type).toBe('heading'); + expect(block.attrs.level).toBe(level); + expect(block.attrs.textAlign).toBe(align); + expect(convertProseMirrorToMarkdown(doc2)).toBe(md1); + }); + } +}); + +describe('attached-comment fail-open in the import pipeline (#293 #9)', () => { + it('a malformed attrs comment is ignored (default attrs kept)', async () => { + const doc2 = await markdownToProseMirror('hello '); + const block = firstBlock(doc2); + expect(block.type).toBe('paragraph'); + expect(block.attrs.textAlign ?? null).toBeNull(); + expect(block.content?.[0]?.text).toBe('hello'); + }); + + it('an unknown key in a valid attrs comment is ignored, no comment leaks', async () => { + const doc2 = await markdownToProseMirror('hello '); + const block = firstBlock(doc2); + expect(block.type).toBe('paragraph'); + expect(block.attrs.textAlign ?? null).toBeNull(); + // The unknown key and the comment marker must not survive into the body. + expect(block.content?.[0]?.text).toBe('hello'); + const serialized = JSON.stringify(doc2); + expect(serialized).not.toContain('bogus'); + expect(serialized).not.toContain('