mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
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()`.
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
@@ -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]
|
||||
}
|
||||
})
|
||||
@@ -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()
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user