From df6ccf2500448ea242885bb53d9b3c2d17a41298 Mon Sep 17 00:00:00 2001 From: averydev Date: Sat, 12 Sep 2026 13:36:38 -0700 Subject: [PATCH] fix(editor): round-trip escapes as atoms and narrow the end-of-file reconcile Review findings on the first commit, all reproduced with the real serializer: - Rewriting marked's `escape` token to text kept the character on load but the serializer never re-escapes, so `\# x` came back as `# x` and became a heading on the next open; `\$x\$` became inline math. Replace the facade rewrite with an inline atom node that owns the escape token and renders `\` + character, so escapes round-trip byte for byte in every container. - The body-strip reconcile ran for every trailing-newline shape. A source ending in a blank line parses to a trailing empty paragraph, so canonical ends in `\n\n` too; stripping and re-attaching there duplicated the paragraph, failed the branch-6 proof, and canonicalized the whole file. Only strip when the source ends in exactly one newline that canonical lacks; every other shape patches correctly whole, as on main. - Canonical fallbacks now keep the source's single trailing newline. - The inline-math regex allowed neither a soft line break inside a formula nor a trailing LaTeX line break; both are valid and upstream accepted them. Drop the newline and backslash exclusions and the dead `.trim()`. --- .../editor/markdown-round-trip.test.ts | 36 +++++++-- .../editor/rich-markdown-escaped-character.ts | 67 +++++++++++++++++ .../editor/rich-markdown-extensions.ts | 3 + .../editor/rich-markdown-inline-math.ts | 5 +- .../rich-markdown-source-reconcile.test.ts | 53 +++++++++++++- .../editor/rich-markdown-source-reconcile.ts | 73 +++++++++++-------- 6 files changed, 195 insertions(+), 42 deletions(-) create mode 100644 src/renderer/src/components/editor/rich-markdown-escaped-character.ts diff --git a/src/renderer/src/components/editor/markdown-round-trip.test.ts b/src/renderer/src/components/editor/markdown-round-trip.test.ts index a47e7819efd..f769ea2510c 100644 --- a/src/renderer/src/components/editor/markdown-round-trip.test.ts +++ b/src/renderer/src/components/editor/markdown-round-trip.test.ts @@ -624,26 +624,50 @@ describe('rich markdown round trip', () => { expect(slashCommandSelectionParent(commandId)).toBe('detailsSummary') }) - it('keeps backslash-escaped characters instead of dropping them on load', () => { - // Why: marked emits `escape` tokens that Tiptap's parser otherwise discards, deleting the character. - expect(roundTripMarkdown('cost was \\$1,200, rate 5\\*, file\\_name, see \\[note\\].\n')).toBe( - 'cost was $1,200, rate 5*, file_name, see [note].' - ) + it('round-trips backslash-escaped characters byte for byte', () => { + // Why: Tiptap's parser dropped marked's `escape` token (the character vanished on load), and its + // serializer never re-escapes, so a bare character would become syntax on the next open. + const content = [ + 'cost was \\$1,200, rate 5\\*, file\\_name, see \\[note\\](y).', + '\\# not a heading', + '1\\. not a list', + '\\- not a bullet', + '\\*\\*not bold\\*\\* and \\_not em\\_', + '\\`not code\\` and \\~\\~not strike\\~\\~', + 'C:\\\\Program Files\\\\(x86)', + 'shell \\$HOME\\$ var' + ].join('\n\n') + const once = roundTripMarkdown(`${content}\n`) + expect(once).toBe(content) + expect(roundTripMarkdown(`${once}\n`)).toBe(content) }) it('keeps escaped dollars inside table cells', () => { - expect(roundTripMarkdown('| Item | Amount |\n|---|---|\n| Fee | \\$500 |\n')).toContain('$500') + expect(roundTripMarkdown('| Item | Amount |\n|---|---|\n| Fee | \\$500 |\n')).toContain( + '\\$500' + ) + }) + + it('does not turn escaped dollars into inline math', () => { + expect(countInlineMathNodes('shell \\$HOME\\$ var and \\$x\\$ too')).toBe(0) }) it('keeps dollar amounts as text instead of inline math', () => { const content = 'from $10 to $20, then (deficit −$509,542 by end-2020) entered 2021 at $0' expect(countInlineMathNodes(content)).toBe(0) expect(roundTripMarkdown(`${content}\n`)).toBe(content) + // Why: Pandoc boundaries — a space inside either `$` or a digit after the closing `$` means money. + for (const text of ['$x$2 apples', 'costs $ x$ here', 'costs $x $ here', '$5-$6 range']) { + expect(countInlineMathNodes(text)).toBe(0) + } }) it('still parses inline math that touches its dollar signs', () => { expect(countInlineMathNodes('Energy is $E = mc^2$ here, and $x_1$ too.')).toBe(2) expect(roundTripMarkdown('Energy is $E = mc^2$ here.\n')).toBe('Energy is $E = mc^2$ here.') + // Why: a formula may wrap across a soft line break, and may end in a LaTeX line break. + expect(countInlineMathNodes('wrap $a +\nb$ end')).toBe(1) + expect(countInlineMathNodes('break $x\\\\$ end')).toBe(1) }) it('preserves markdown tables', () => { diff --git a/src/renderer/src/components/editor/rich-markdown-escaped-character.ts b/src/renderer/src/components/editor/rich-markdown-escaped-character.ts new file mode 100644 index 00000000000..17b43cbf8ff --- /dev/null +++ b/src/renderer/src/components/editor/rich-markdown-escaped-character.ts @@ -0,0 +1,67 @@ +import { Node } from '@tiptap/core' + +// marked's GFM inline escape rule: a backslash before ASCII punctuation. +const ESCAPED_CHARACTER_PATTERN = /^\\([!"#$%&'()*+,\-./:;<=>?@[\]\\^_`{|}~])/ +const NODE_NAME = 'richMarkdownEscapedCharacter' +const MARKER_ATTRIBUTE = 'data-rich-markdown-escaped-character' + +/** + * One inline atom per backslash-escaped character. Tiptap's markdown parser has + * no handler for marked's `escape` token (the character was deleted on load), + * and its text serializer never re-escapes, so a text node could not carry the + * backslash back to disk. An atom that owns the token round-trips `\$` as `\$`. + */ +export const RichMarkdownEscapedCharacter = Node.create({ + name: NODE_NAME, + inline: true, + group: 'inline', + atom: true, + // Why: reads as one character of text; a node selection on click would be noise. + selectable: false, + + addAttributes() { + return { + character: { default: '', rendered: false } + } + }, + + markdownTokenName: NODE_NAME, + markdownTokenizer: { + name: NODE_NAME, + level: 'inline', + start: (src: string) => src.indexOf('\\'), + tokenize(src: string) { + const match = src.match(ESCAPED_CHARACTER_PATTERN) + if (!match) { + return undefined + } + return { type: NODE_NAME, raw: match[0], character: match[1] } + } + }, + parseMarkdown: (token, helpers) => { + const character = (token as { character?: string }).character + if (token.type !== NODE_NAME || !character) { + return [] + } + return helpers.createNode(NODE_NAME, { character }) + }, + renderMarkdown: (node) => `\\${String(node.attrs?.character ?? '')}`, + renderText: ({ node }) => String(node.attrs.character ?? ''), + + parseHTML() { + return [ + { + tag: `span[${MARKER_ATTRIBUTE}]`, + getAttrs: (element: HTMLElement) => { + const character = element.getAttribute(MARKER_ATTRIBUTE) ?? '' + return ESCAPED_CHARACTER_PATTERN.test(`\\${character}`) ? { character } : false + } + } + ] + }, + + renderHTML({ node }) { + const character = String(node.attrs.character ?? '') + return ['span', { [MARKER_ATTRIBUTE]: character }, character] + } +}) diff --git a/src/renderer/src/components/editor/rich-markdown-extensions.ts b/src/renderer/src/components/editor/rich-markdown-extensions.ts index 398ba855684..77cee516456 100644 --- a/src/renderer/src/components/editor/rich-markdown-extensions.ts +++ b/src/renderer/src/components/editor/rich-markdown-extensions.ts @@ -43,6 +43,7 @@ import { RichMarkdownProseEntities } from './rich-markdown-prose-entities' import { RichMarkdownParagraph } from './rich-markdown-paragraph' import { RichMarkdownInlineMath } from './rich-markdown-inline-math' import { RichMarkdownCodeBlockLowlight } from './rich-markdown-lowlight' +import { RichMarkdownEscapedCharacter } from './rich-markdown-escaped-character' import { RichMarkdownTaskList } from './rich-markdown-task-list' import { createCachedLowlight } from './rich-markdown-lowlight-cache' import { renderRichMarkdownCodeBlock } from './rich-markdown-code-block-markdown' @@ -248,6 +249,7 @@ export function createRichMarkdownExtensions({ throwOnError: false } }), + RichMarkdownEscapedCharacter, createRichMarkdownLiteral(codec.transport), ...(htmlSuperscriptLinks ? [createRichMarkdownHtmlSuperscriptLink(codec.transport, htmlSuperscriptLinkContext!)] @@ -262,6 +264,7 @@ export function createRichMarkdownExtensions({ gfm: true } }), + RichMarkdownEscapedCharacter, RichMarkdownCodeSpanPadding, RichMarkdownProseEntities, createRichMarkdownAnnotationHighlightExtension() diff --git a/src/renderer/src/components/editor/rich-markdown-inline-math.ts b/src/renderer/src/components/editor/rich-markdown-inline-math.ts index 7b197e5d2b3..05f6d32fda3 100644 --- a/src/renderer/src/components/editor/rich-markdown-inline-math.ts +++ b/src/renderer/src/components/editor/rich-markdown-inline-math.ts @@ -1,8 +1,7 @@ import { InlineMath } from '@tiptap/extension-mathematics' -// Why: the common dialect requires a non-space next to each delimiter and forbids a -// newline inside, which is what keeps `US$ 5,000 and R$ 40,000` out of a math span. -const INLINE_MATH = /^\$(?![\s$])((?:[^$\n]*[^\s$])?)\$(?![\d$])/ +// Keep money as text while allowing valid multiline and escaped LaTeX content. +const INLINE_MATH = /^\$(?![\s$])([^$]*?[^\s$])\$(?![\d$])/ const baseTokenizer = InlineMath.config.markdownTokenizer if (!baseTokenizer) { diff --git a/src/renderer/src/components/editor/rich-markdown-source-reconcile.test.ts b/src/renderer/src/components/editor/rich-markdown-source-reconcile.test.ts index 44a42cd0ec4..da3106b442b 100644 --- a/src/renderer/src/components/editor/rich-markdown-source-reconcile.test.ts +++ b/src/renderer/src/components/editor/rich-markdown-source-reconcile.test.ts @@ -406,7 +406,8 @@ describe('serializeRichMarkdownForReconcile (real editor pipeline)', () => { const originalSource = 'Cost was \\$1,200 for Nell & Mary.\n\n| Item | Amount |\n|--------------|-------:|\n| Fee | \\$500 |\n\nTrailing paragraph.\n' const baseCanonical = serialize(originalSource)! - expect(baseCanonical.endsWith('\n')).toBe(false) // getMarkdown never emits a trailing newline + // getMarkdown has no trailing newline for a source that ends in a single newline + expect(baseCanonical.endsWith('\n')).toBe(false) const edited = `${baseCanonical} Added word.` const reconciled = reconcileSerializedMarkdown({ @@ -421,6 +422,39 @@ describe('serializeRichMarkdownForReconcile (real editor pipeline)', () => { ) }) + it('keeps source style when typing into a trailing empty paragraph (source ends in a blank line)', () => { + // Why: `text\n\n` parses to a trailing empty paragraph, so canonical ends in `\n\n` too; the + // end-of-file body strip must not run here or the whole file falls back to canonical. + const originalSource = 'Cost \\$1 for _em_.\n\ntext\n\n' + const baseCanonical = serialize(originalSource)! + expect(baseCanonical.endsWith('\n\n')).toBe(true) + const edited = `${baseCanonical}Added` + + const reconciled = reconcileSerializedMarkdown({ + originalSource, + baseCanonical, + edited, + roundTrip: (md) => serialize(md) + }) + + expect(reconciled).toBe('Cost \\$1 for _em_.\n\ntext\n\nAdded') + }) + + it('keeps source style when deleting the trailing empty paragraph', () => { + const originalSource = 'Cost \\$1 for _em_.\n\ntext\n\n' + const baseCanonical = serialize(originalSource)! + const edited = baseCanonical.replace(/\n+$/, '') // Backspace at end of document + + const reconciled = reconcileSerializedMarkdown({ + originalSource, + baseCanonical, + edited, + roundTrip: (md) => serialize(md) + }) + + expect(reconciled).toBe('Cost \\$1 for _em_.\n\ntext') + }) + it('applies normalizeEmptyListItems so empty list items round-trip stably', () => { // `3. ` immediately before a heading parses as an empty list item; without the // normalize step the safety re-parse would spuriously mismatch and no-op. @@ -743,6 +777,23 @@ describe('reconcileSerializedMarkdown end-of-document edits', () => { expect(reconciled).toBe('# Title\r\n\r\n_emphasis_\r\n\r\nTrailing paragraph. Added word.\r\n') }) + it('keeps the source trailing newline when falling back to canonical output', () => { + // Branch 3 (oversize) is the simplest guaranteed fallback. + const body = '# Title\n\n_emphasis_\n\n'.repeat(4_000) + const originalSource = `${body}Trailing paragraph.\n` + const baseCanonical = canonicalWithoutTrailingNewline(originalSource) + const edited = `${baseCanonical} Added word.` + + const reconciled = reconcileSerializedMarkdown({ + originalSource, + baseCanonical, + edited, + roundTrip: canonicalWithoutTrailingNewline + }) + + expect(reconciled).toBe(`${edited}\n`) + }) + it('still patches an edit at the end of a source without a trailing newline', () => { const originalSource = '# Title\n\n_emphasis_\n\nTrailing paragraph.' const baseCanonical = canonicalWithoutTrailingNewline(originalSource) diff --git a/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts b/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts index b67b0c0c96f..370988eb2eb 100644 --- a/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts +++ b/src/renderer/src/components/editor/rich-markdown-source-reconcile.ts @@ -30,12 +30,18 @@ export type ReconcileSerializedMarkdownParams = { roundTrip: (markdown: string) => string | null } -/** Restores the source's EOL sequence and final newline to canonical markdown. */ +export function restoreMarkdownSourceEol(markdown: string, source: string): string { + return restoreEol(toLf(markdown), detectDominantEol(source)) +} + +/** Restores canonical output to the source's EOL and single trailing newline. */ export function restoreMarkdownSourceLineEndings(markdown: string, source: string): string { - return restoreEol( - preserveSourceTrailingNewline(toLf(markdown), toLf(source)), - detectDominantEol(source) - ) + const sourceLf = toLf(source) + const markdownLf = toLf(markdown) + const sourceTrailing = sourceLf.match(/\n+$/)?.[0] ?? '' + const withTrailing = + markdownLf.endsWith('\n') || sourceTrailing.length !== 1 ? markdownLf : `${markdownLf}\n` + return restoreEol(withTrailing, detectDominantEol(source)) } /** @@ -69,35 +75,45 @@ export function reconcileSerializedMarkdown({ return restoreEol(editedLf + originalTrailingNewlines, eol) } + // Why: canonical output has no final newline; keep the source's single one so a fallback does not strip it. + const canonicalFallback = (): string => + restoreEol( + editedLf.endsWith('\n') || originalTrailingNewlines.length !== 1 + ? editedLf + : editedLf + originalTrailingNewlines, + eol + ) + // Branch 3: oversize → bounded-cost canonical fallback (today's behavior). if ( Math.max(originalSource.length, baseCanonical.length, edited.length) > RECONCILE_SIZE_CAP_CODE_UNITS ) { - return restoreMarkdownSourceLineEndings(edited, originalSource) + return canonicalFallback() } // Branch 4: run the divergent-base patch entirely in LF space. - // Why: dmp's half-match accelerator ignores the diff deadline (100ms+ on repeated seeds), so bail to canonical for highly repetitive replacements. - if (hasRepeatedHalfMatchSeed(baseLf, editedLf)) { - return restoreMarkdownSourceLineEndings(edited, originalSource) - } - - // Patch newline-stripped bodies so an EOF edit cannot land after the source's - // non-semantic trailing newline and force a whole-file canonical rewrite. - const sourceBody = stripTrailingNewlines(originalSourceLf) - const baseBody = stripTrailingNewlines(baseLf) - const editedBody = stripTrailingNewlines(editedLf) - const sourceTrailingNewlines = originalSourceLf.match(/\n+$/)?.[0] ?? '' - const baseTrailingNewlines = baseLf.match(/\n+$/)?.[0] ?? '' + // Why: when the source ends in one newline that canonical lacks, an end-of-document hunk (whose + // trailing context is dmp's end-of-text padding) lands after that newline and fails branch 6, so + // patch the bodies and re-attach the run. Any other shape (a trailing empty paragraph is `\n\n` + // in canonical too) patches correctly whole. const editedTrailingNewlines = editedLf.match(/\n+$/)?.[0] ?? '' - const reconciledTrailingNewlines = - (editedTrailingNewlines === baseTrailingNewlines ? '' : editedTrailingNewlines) + - sourceTrailingNewlines + const stripEnd = !baseLf.endsWith('\n') && originalTrailingNewlines.length === 1 + const sourceBody = stripEnd ? stripTrailingNewlines(originalSourceLf) : originalSourceLf + const baseBody = baseLf + const editedBody = stripEnd ? stripTrailingNewlines(editedLf) : editedLf + const reconciledTrailingNewlines = stripEnd + ? editedTrailingNewlines + originalTrailingNewlines + : '' + // Why: dmp's half-match accelerator ignores the diff deadline (100ms+ on repeated seeds), so bail to canonical for highly repetitive replacements. + if (hasRepeatedHalfMatchSeed(baseBody, editedBody)) { + return canonicalFallback() + } let diffs = makeDiff(baseBody, editedBody, { checkLines: true, timeout: RECONCILE_DIFF_TIMEOUT_SECONDS }) + // Match makePatches's cleanup while supplying our own bounded diff, avoiding the library's 1s timeout. if (diffs.length > 2) { diffs = cleanupSemantic(diffs) diffs = cleanupEfficiency(diffs) @@ -117,24 +133,17 @@ export function reconcileSerializedMarkdown({ // Branch 5: a hunk failed to locate in the non-canonical source → unreliable fuzzy match, fall back to canonical. if (results.some((applied) => !applied)) { - return restoreMarkdownSourceLineEndings(edited, originalSource) + return canonicalFallback() } // Branch 6: prove reconciled bytes render-equal the editor's document — any fuzzy misplacement changes canonical output and is caught here → canonical fallback. const reparsed = roundTrip(reconciledLf) if (reparsed === null || normalizeForSafety(reparsed) !== normalizeForSafety(editedLf)) { - return restoreMarkdownSourceLineEndings(edited, originalSource) + return canonicalFallback() } - return restoreEol(preserveSourceTrailingNewline(reconciledLf, originalSourceLf), eol) -} - -function preserveSourceTrailingNewline(lfText: string, originalSourceLf: string): string { - // A single final newline is non-semantic; longer runs can create an empty paragraph. - if (lfText.endsWith('\n') || !originalSourceLf.endsWith('\n')) { - return lfText - } - return `${lfText}\n` + // Restore the detected EOL as the final step so reconciled CRLF stays CRLF. + return restoreEol(reconciledLf, eol) } function stripTrailingNewlines(lfText: string): string {