Repository navigation
Conversation
|
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 2 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="packages/editor/src/core/markdown/MarkdownSerializer.ts" line_range="455" />
<code_context>
- const [_, rest, trail] = /^(.*?)(\s*)$/m.exec(node.text!)!;
+ // Read all lines only for one escaping mark in a block.
+ const trailingWhitespace = singleEscapedMark && fromBlockStart
+ ? /^([\s\S]*\S)?(\s*)$/ : /^(.*?)(\s*)$/m;
+ const [_, rest = '', trail] = trailingWhitespace.exec(node.text!)!;
if (trail) {
</code_context>
<issue_to_address>
**Trailing whitespace stays inside the ending mark**
When a multiline text node ends with whitespace inside an escaping mark or immediately before an unmarked break, `trailingWhitespace` falls back to the line-limited `/m` regex, which can stop at the first line and miss whitespace at the end of the text node. The whitespace remains inside the ending mark, so Markdown parsing places the emphasis and whitespace incorrectly.
Use a multiline-aware regex to extract trailing whitespace from the end of the text node, including when `fromBlockStart` is false.
</issue_to_address>
### Comment 2
<location path="packages/editor/src/core/markdown/MarkdownSerializer.whitespace.test.ts" line_range="234" />
<code_context>
+ }).buildDeps();
+ const marks = markOrder.reduce(
+ (ordered, name) => ordered.addToEnd(name, deps.schema.marks[name].spec),
+ deps.schema.spec.marks,
+ );
+ const schema = new Schema<string, string>({nodes: deps.schema.spec.nodes, marks});
</code_context>
<issue_to_address>
**Alternate mark orders are not exercised**
When a test calls `createFixture` with a `markOrder` that differs from the original schema order, when `createFixture` receives a `markOrder` different from the schema’s original order, `OrderedMap.addToEnd` updates the existing entries in place rather than moving them. The tests therefore still serialize using the original mark order and can pass while reordered-mark behavior for the requested orders is broken.
Construct the marks map in the requested order, rather than calling `addToEnd` on entries that already exist.
Also at `packages/editor/src/core/markdown/MarkdownSerializer.whitespace.research.test.ts:453`.
</issue_to_address>
|
|
e2ba70a to
200ace0
Compare
| if ( | ||
| !node && | ||
| noEsc && | ||
| active.every((mark) => this.getMark(mark.type.name).expelEnclosingWhitespace) |
There was a problem hiding this comment.
The forced keep = 0 closes marks where the closing delimiter is not right-flanking or the marks continue on the next node, so the marks turn into literal text.
doc(p(strong('x '), strong(code(' ')), em(strong('y'))))
// actual: **x ** ***y*** -> p("**x ** ", em(strong("y")))
// expected: **x *y*** -> p(strong("x "), em(strong("y")))
doc(p(strong('a'), strong(em('x')), em(strong(code(' '))), em(strong('y'))))
// actual: **a*x*** ***y*** (the space loses both marks)
// expected: **a*x y***
doc(p(em(img()), em(strong(code('\u2028'))), 'y'))
// actual: **\u2028y -> p("*", image, "*\u2028y")
// expected: *\u2028*y -> p(em(image), em("\u2028"), "y")All three are regressions: main returns the expected output for each. The condition checks neither the character before the delimiter nor the marks of the next node, and isMarkAhead ignores the reordering of mixable marks.
Closing only marks that end here, with a right-flanking delimiter, fixes all three and keeps em('x'), strong(em(code(' '))) → *x* :
// Close marks before expelled whitespace when they end here and the delimiter stays right-flanking.
if (
!node &&
noEsc &&
/^[\t\n\f\r\p{Zs}]/u.test(leading + trailing) &&
!/[\t\n\f\r\p{Zs}]$/u.test(this.out) &&
!active.every((mark) => mark.isInSet(next?.marks ?? [])) &&
active.every((mark) => this.getMark(mark.type.name).expelEnclosingWhitespace)
) keep = 0;Each of the three new conditions alone covers only some of the inputs. src/core/markdown with this change: 193 passed, 12 expected fail, 5 skipped.
There was a problem hiding this comment.
Not fixed yet. I reproduced all three regressions. The guarded closing variants fix those examples, but they also break previously valid output. I removed the unsafe candidates from the working changes.
Counterexample for the closing guard
Mark order: em, link, strong, code. The link serializer is:
{open: '[', close: '](foo)', expelEnclosingWhitespace: true}const left = strong('x ');
const space = em(strong(code(' ')));
const right = em(a(strong(' y')));
const input = doc(p(left, space, right));Published output:
**x** *[**y**](foo)*The guarded candidate produces:
**x ** *[**y**](foo)*The first strong mark becomes literal **x ** after parsing.
Checking only the marks on the current and next nodes is not enough. Closing also depends on the emitted delimiter order and surrounding characters. I have not found a safe small fix; the next step is to agree a wider change that separates source marks from the open delimiter stack.
There was a problem hiding this comment.
Update: the first two text examples are fixed in f37766f. They keep bold and italic after parsing. With the default preset, the code space can also keep its code mark, so the Markdown may differ from the proposed normalized output.
The image with U+2028 still fails. The scoped fixes do not cover that boundary case, so this part remains open:
doc(p(em(img()), em(strong(code('\u2028'))), 'y'))
// Markdown: **\u2028y
// Parsed: literal '*', image, literal '*\u2028y'|
|
||
| // Keep whitespace-only code outside reordered marks. | ||
| if (reorderedNoEsc && whitespaceOnly) { | ||
| marks = []; |
There was a problem hiding this comment.
marks = [] also removes marks shared with active that continue after the code, so a continuing mark is split around it.
// fixture with StrikeSpecs and s: {markType: 'strike'}
doc(p(s(strong(em(a('x')))), s(a(code(' '))), s(a('y'))))
// actual: ~~***[x](foo)***~~` `~~[y](foo)~~ -> code(" ") without strike
// expected: strike(code(" "))Regression for strike: main keeps strike(code(" ")) and loses only link.
Keeping the prefix that matches active and continues past the node fixes it:
// Keep whitespace-only code only inside open marks that continue past it.
if (reorderedNoEsc && whitespaceOnly) {
len = 0;
while (
len < Math.min(active.length, marks.length) &&
marks[len].eq(active[len]) &&
this.isMarkAhead(parent, index + 1, active.slice(0, len + 1))
) ++len;
marks = marks.slice(0, len);
}Output: ~~***[x](foo)***` `[y](foo)~~ → strike(code(" ")). src/core/markdown: 193 passed, 12 expected fail, 5 skipped.
There was a problem hiding this comment.
Not fixed yet. Clearing all marks removes the continuing strike mark. Keeping the matching prefix fixes the reported strike example, but it can keep em open across code and break a later strong delimiter. I removed that candidate from the working changes.
Counterexample for keeping the continuing prefix
The fixture includes StrikeSpecs, with s bound to strike. Mark order: em, strong, link, strike, code.
const left = em(strong(a(s('x'))));
const space = em(s(code(' ')));
const right = em(strong(a('y')));
const input = doc(p(left, space, right));Published output:
***[~~x~~](foo)***` `***[y](foo)***The prefix-preserving candidate produces:
***[~~x~~](foo)**` `**[y](foo)***After parsing, y loses strong and literal ** appears at the end.
Further continuity guards still left Unicode and custom-spec counterexamples. Both strike and em/strong have mixable: true and expelEnclosingWhitespace: true, so those flags cannot distinguish safe cases. This remains open while we agree the broader mark-transition fix.
There was a problem hiding this comment.
Update: fixed for DefaultSpecsPreset in f37766f. The shared link and strike marks now stay on the code space. Added a round-trip test:
[***~~x~~***~~` `y~~](foo)The case still fails with mark order strike, em, strong, link, code: the code space loses strike. The current guard preserves shared marks only when the outer mark allows whitespace. The broader prefix change still has counterexamples, so this thread remains open for other mark orders.
200ace0 to
5ac54a7
Compare
|
|
||
| if ( | ||
| !node && | ||
| noEsc && |
There was a problem hiding this comment.
Saving a loaded document turns bold, italic or strikethrough before a code-formatted space into literal *, **, ~~.
Input: *x` `**y**.* (also ~~x ` `[y](foo)~~).
This branch: *x* ***y****.* → em("x"), " ", strong(em("y")), "*.*"; main: *x **y**.*, marks kept.
Force-close only when expelled whitespace leads and the output does not already end with whitespace:
| noEsc && | |
| noEsc && | |
| leading && | |
| !/\s/.test(this.out.slice(-1)) && |
With both fixes: 4 such inputs from markdown match main, your :474 counterexample stays **x** *[**y**](foo)*; src/core/markdown: 206 passed, 11 expected fail, 5 skipped.
There was a problem hiding this comment.
Fixed in f37766f. Both loaded examples now keep emphasis and strikethrough as Markdown marks, without literal delimiters. I narrowed the forced-close condition and added regression tests.
*x` `**y**.* -> *x **y**.*
~~x ` `[y](foo)~~ -> ~~x [y](foo)~~
| } | ||
|
|
||
| // Keep whitespace-only code outside reordered marks. | ||
| if (reorderedNoEsc && whitespaceOnly) { |
There was a problem hiding this comment.
Saving a loaded document merges a code-formatted space with the next inline code and puts backticks into the code text.
Input: [***x***](foo)*` `*`x` .
This branch: [***x***](foo)` x`` →code(" x")`; `main`: x x ``, text kept.
Keep the space inside the open marks when the next node is code:
| if (reorderedNoEsc && whitespaceOnly) { | |
| if ( | |
| reorderedNoEsc && | |
| whitespaceOnly && | |
| !parent.maybeChild(index + 1)?.marks.some((m) => this.getMark(m.type.name).escape === false) | |
| ) { |
With both fixes: 2 such inputs match main, your :419 counterexample stays ***[~~x~~](foo)***` `***[y](foo)***; src/core/markdown: 206 passed, 11 expected fail, 5 skipped.
There was a problem hiding this comment.
Fixed in f37766f. The loaded example now serializes as:
[***x*** ](foo)`x`The last x stays in one code node, without extra backticks in its text. Added tests for this case and nested marks after a code space.
Preserve continuing emphasis, links, and inline code around whitespace. Close ending marks after final expelled spaces and track known custom-spec limits. Skip unused whitespace checks when no marks need closing.
f37766f to
2d79736
Compare
| this.isMarkAhead(parent, index + 1, sourceMarks.slice(0, -1)); | ||
| // Keep whitespace-only code outside reordered marks. | ||
| if (reorderedNoEsc && whitespaceOnly && !( | ||
| !this.getMark(active[0].type.name).expelEnclosingWhitespace && |
There was a problem hiding this comment.
[P2] Preserve boundaries between adjacent code spans
packages/editor/src/core/markdown/MarkdownSerializer.ts:429
With DefaultSpecsPreset, loading and saving either input adds two literal backticks to the document text after parsing:
const ascii = '~~***x***` `~~`y`';
const unicode = '[***x***](foo)*`\u00a0`*`y`';The first input saves as:
***~~x~~***` ``y`Parsing produces code(" ``y") instead of a final code("y"). The base preserves the text for both inputs. The guard excludes an outer strong mark in the first case and NBSP in the second. Both paths clear the whitespace node's marks and emit adjacent code delimiters. Please preserve that boundary for both cases.
There was a problem hiding this comment.
Fixed in bd5115a3. When reordered marks are removed, whitespace before the next code span is written without code delimiters. Both ASCII-space and NBSP cases now keep the next code text unchanged. Added round-trip tests, including a next code span with literal backticks.
| else if (j > i) { | ||
| marks = marks.slice(0, i).concat(marks.slice(i + 1, j)).concat(mark).concat(marks.slice(j, len)) | ||
| if (noEsc && j > len) reorderedNoEsc = true; | ||
| const end = this.getMark(mark.type.name).expelEnclosingWhitespace ? j : Math.min(j, len); |
There was a problem hiding this comment.
end is limited by len only for a mark without expelEnclosingWhitespace (the link). For em, strong and strike the slice still takes the code mark, code is opened as a plain delimiter, and the outer mark is lost on the code.
Input (DefaultSpecsPreset): ***~~x~~***~~`y`~~ → ***~~x~~***``y`` → code("y") without strike. Also [***x***](foo)*`y`* (em lost) and [**~~x~~**](foo)~~`x`~~.
main gives the same output, but this is the strike/em and code overlap the PR targets.
| const end = this.getMark(mark.type.name).expelEnclosingWhitespace ? j : Math.min(j, len); | |
| const end = Math.min(j, len); |
With it all three inputs round-trip; src/core/markdown: 233 passed, 11 expected fail, 5 skipped; counterexamples from threads 3, 5, 7, 8 are unchanged.
There was a problem hiding this comment.
Fixed in bd5115a3. For text nodes, the reordered prefix is now limited to len, so the raw code mark cannot replace em/strong/strike. Non-text nodes keep the previous behavior. All three examples now round-trip; added regression tests.
Custom inline-code specs with empty open/close can still produce literal delimiters. We accepted this limitation and kept the examples as research cases, outside the regression suite.
| } | ||
|
|
||
| const expelBeforeBreak = !noEsc && marks.every(mark => this.getMark(mark.type.name).expelEnclosingWhitespace) && | ||
| !/[\r\n\u2028\u2029]/.test(node?.text ?? '') && next?.type.spec.isBreak; |
There was a problem hiding this comment.
Bold, italic or strike that ends with a code-formatted space after a space turns into literal text (the expel is at :461-466, below this line).
Input (DefaultSpecsPreset): **x ` `** → **x ** → p("**x **"); also *`d` ` `*, ~~`d` ` `~~, # **x ` `**.
The space of x stays inside strong because the next node has it; the code space is then expelled, none of the alternatives at :486-489 apply, and ** closes after a space. main gives the same output.
Fix: keep the code space inside the marks when the output already ends with a space:
+ // Keep whitespace-only code inside marks when the output already ends with a space.
+ const keepCodeWhitespace = noEsc && whitespaceOnly && /[ \t]$/.test(this.out) &&
+ !/[\r\n\u2028\u2029]/.test(node?.text ?? '') &&
+ (!next || next.type.spec.isBreak || (next.isText && /^\s/.test(next.text ?? '')) ||
+ this.isMarkAhead(parent, index + 1, active));
- if (node && node.isText && marks.some((mark, i) => {
+ if (node && node.isText && !keepCodeWhitespace && marks.some((mark, i) => {With it all four inputs round-trip; src/core/markdown: 233 passed, 11 expected fail, 5 skipped; counterexamples from threads 3, 5, 7, 8 are unchanged.
There was a problem hiding this comment.
Fixed in bd5115a3. Code-only spaces now stay inside the outer mark when expelling them would leave its closing delimiter after a space. The guard applies only to code specs that do not request whitespace expulsion, and runs only when trailing whitespace would be expelled. Added tests for bold, italic, strike, headings, and soft/hard breaks.
| sourceMarks.slice(0, -1).every(mark => mark.isInSet(active)) && | ||
| this.isMarkAhead(parent, index + 1, sourceMarks.slice(0, -1)); | ||
| // Keep whitespace-only code outside reordered marks. | ||
| if (reorderedNoEsc && whitespaceOnly && !( |
There was a problem hiding this comment.
Regression: whitespace-only code still merges with the next inline code when the first open mark is not a link, or when the space is NBSP. The next code gets literal backticks in its text.
default preset
~~***a***` `~~`b` -> ***~~a~~***` ``b` -> code(" ``b")
main -> ***~~a~~*** `b` -> " ", code("b")
[***x***](foo)*`<NBSP>`*`x` -> [***x***](foo)`<NBSP>``x` -> code("<NBSP>``x")
main -> [***x***<NBSP>](foo)`x`
The exception in this condition covers only a non-expelling active[0] and spaces or tabs. Skipping the reset when the next node is the same code with non-empty delimiters fixes both:
this.isMarkAhead(parent, index + 1, sourceMarks.slice(0, -1));
+ // Keep whitespace-only code from joining the delimiters of the next code node.
+ const joinsNextCode = noEsc && next?.isText && next.marks.length === 1 && next.marks[0].eq(inner) &&
+ this.markString(inner, true, parent, index + 1) !== '' &&
+ (!this.getMark(inner.type.name).expelEnclosingWhitespace || /^\S/.test(next.text ?? ''));
// Keep whitespace-only code outside reordered marks.
- if (reorderedNoEsc && whitespaceOnly && !(
+ if (reorderedNoEsc && whitespaceOnly && !joinsNextCode && !(With it both inputs give the main output; src/core/markdown: 233 passed, 11 expected fail, 5 skipped; tsc clean; your counterexamples from the threads above are unchanged.
There was a problem hiding this comment.
Fixed in bd5115a3. I kept the mark reset and omit the current whitespace code delimiters at output instead. This fixes both cases without an extra call to a custom open callback. NBSP stays outside the previous link, as in the input. Added round-trip tests for both cases and code containing backticks.
| // Read all lines when ending marks cannot affect adjacent nodes. | ||
| const trailingWhitespace = ((!noEsc && marks.length === 1) || isolatedEscapingMarks) | ||
| ? /^([\s\S]*\S)?(\s*)$/ : /^(.*?)(\s*)$/m; | ||
| const [_, rest = '', trail] = trailingWhitespace.exec(node.text!)!; |
There was a problem hiding this comment.
Text after U+2028/U+2029 is dropped when an outer mark continues. main drops it too, so this is not a regression, but it is text loss in the code this PR changes.
default preset, U+2028 after the space
*a **x <U+2028>y*** -> *a **x*** -> em("a "), strong(em("x")) // "y" is lost
# *a **x <U+2028>y*** -> # *a **x***
[a *x <U+2028>y*](foo) -> [a *x*](foo)
The /m match stops at the first line separator, and withText(rest) drops the remaining lines. Falling back to the whole-text pattern when the line match ends early keeps the text:
| const [_, rest = '', trail] = trailingWhitespace.exec(node.text!)!; | |
| const lineMatch = trailingWhitespace.exec(node.text!)!; | |
| // A line match that ends before the text end would drop the following lines. | |
| const [_, rest = '', trail] = lineMatch[2] && lineMatch[0].length < node.text!.length | |
| ? /^([\s\S]*\S)?(\s*)$/.exec(node.text!)! : lineMatch; |
With it all three inputs round-trip unchanged; src/core/markdown: 233 passed, 11 expected fail, 5 skipped.
There was a problem hiding this comment.
Fixed in bd5115a3. Added the guarded whole-text fallback. It runs only when the line match has trailing whitespace and stops before the end of the text, so the suffix is no longer dropped. Added round-trip tests for U+2028/U+2029 in paragraphs, headings and links, plus raw LF/CR/CRLF cases.
| }); | ||
| // Read all lines when ending marks cannot affect adjacent nodes. | ||
| const trailingWhitespace = ((!noEsc && marks.length === 1) || isolatedEscapingMarks) | ||
| ? /^([\s\S]*\S)?(\s*)$/ : /^(.*?)(\s*)$/m; |
There was a problem hiding this comment.
\s in these patterns (and in the fallback at :487) matches U+2028/U+2029, which CommonMark does not treat as whitespace. Expelling them puts the closing delimiter between punctuation and the separator, where it no longer closes.
Input (DefaultSpecsPreset, from markdown): **x<U+2028>y!<U+2028>**z; also ~~x<U+2028>y!<U+2028>~~z, # **x<U+2028>y!<U+2028>**z, U+2029.
This branch and 2d79736: **x<U+2028>y!**<U+2028>z → p("**x<U+2028>y!**<U+2028>z"), bold turns into literal **. main keeps the input unchanged and it round-trips, because the /m pattern stopped at the first line. A single-line **x y!<U+2028>**z fails on main too.
Keep line and paragraph separators before the closing delimiter:
const trailingWhitespace = ((!noEsc && marks.length === 1) || isolatedEscapingMarks)
- ? /^([\s\S]*\S)?(\s*)$/ : /^(.*?)(\s*)$/m;
+ ? /^([\s\S]*?)([^\S\p{Zl}\p{Zp}]*)$/u : /^(.*?)([^\S\p{Zl}\p{Zp}]*)$/mu;
@@
const [_, rest = '', trail] = lineMatch[2] && lineMatch[0].length < text.length
- ? /^([\s\S]*\S)?(\s*)$/.exec(text)! : lineMatch;
+ ? /^([\s\S]*?)([^\S\p{Zl}\p{Zp}]*)$/u.exec(text)! : lineMatch;With it, the 10 single- and multi-line U+2028/U+2029 inputs round-trip; src/core/markdown: 254 passed, 11 expected fail, 5 skipped; the new marks.test.ts cases pass; your counterexamples **x** *[**y**](foo)*, ***[~~x~~](foo)***` `***[y](foo)*** and ** y are unchanged.
| }); | ||
| // Keep code spaces when an outer mark would close after a space. | ||
| const keepCodeWhitespace = expelTrailingWhitespace && noEsc && whitespaceOnly && inner.type.spec.code === true && | ||
| !this.getMark(inner.type.name).expelEnclosingWhitespace && /[ \t]$/.test(this.out) && |
There was a problem hiding this comment.
The fix for the comment at :461 covers a code space only when the next node is missing, a break, text starting with whitespace, or text with the same marks. Before punctuation, a link or inline code, the mark still turns into literal text.
Input (DefaultSpecsPreset, from markdown): **x ` `**. → **x ** . → p("**x ** ."); also *`d` ` `*!, **x ` `**[y](u), Use **`npm` ` `**, then run.. With NBSP before the code (**x<NBSP>` `**) or without a space (*x` `* → *x* ), /[ \t]$/ skips the guard and the code space is dropped. main and 2d79736 give the same output.
A delimiter right after the closing backtick is right-flanking, so it closes before punctuation and at the end:
- !this.getMark(inner.type.name).expelEnclosingWhitespace && /[ \t]$/.test(this.out) &&
+ !this.getMark(inner.type.name).expelEnclosingWhitespace &&
!/[\r\n\u2028\u2029]/.test(node?.text ?? '') &&
(!next || next.type.spec.isBreak || (next.isText && /^\s/.test(next.text ?? '')) ||
- this.isMarkAhead(parent, index + 1, active));
+ this.isMarkAhead(parent, index + 1, active) ||
+ // A delimiter after the closing backtick still closes before punctuation.
+ (next.isText && next.marks.every(mark => mark.isInSet(active) ||
+ !this.getMark(mark.type.name).expelEnclosingWhitespace) &&
+ (next.marks.some(mark => !mark.isInSet(active)) ||
+ /^(?:[!-/:-@[-`{-~]|\p{P})/u.test(next.text ?? ''))));With it, 14 such inputs round-trip; your counterexamples **x** *[**y**](foo)* and ***[~~x~~](foo)***` `***[y](foo)*** and the new marks.test.ts cases are unchanged. src/core/markdown: 253 passed, 1 failed, 11 expected fail, 5 skipped. The failed test is should preserve loaded strike before a code space and a link (whitespace.test.ts:203): it expects ~~x [y](foo)~~ with the code mark dropped, and the change returns the input ~~x ` `[y](foo)~~ unchanged. The first line alone fixes the NBSP and no-space inputs with no test changes.
| else if (j > i) { | ||
| marks = marks.slice(0, i).concat(marks.slice(i + 1, j)).concat(mark).concat(marks.slice(j, len)) | ||
| if (noEsc && j > len) reorderedNoEsc = true; | ||
| const end = this.getMark(mark.type.name).expelEnclosingWhitespace && !node?.isText |
There was a problem hiding this comment.
For non-text nodes end = j still moves the code mark into marks, so an image with code after reordered marks is written between backticks and comes back as code text.
Input (DefaultSpecsPreset, built; in the editor: apply inline code to an image after bold italic struck text): doc(p(strong(em(s('x'))), s(code(img())))) → ***~~x~~***`` → code(""), the image is gone. main and 2d79736 give the same output. Without reordering, p(s('x'), s(code(img()))) → ~~x~~ keeps the image.
- const end = this.getMark(mark.type.name).expelEnclosingWhitespace && !node?.isText
- ? j : Math.min(j, len);
+ const end = Math.min(j, len);With it: ***~~x~~***~~~~, the image is kept and only the code mark is lost, as without reordering. src/core/markdown: 254 passed, 11 expected fail, 5 skipped; the new marks.test.ts cases and your counterexamples are unchanged.
| if (noEsc && j > len) reorderedNoEsc = true; | ||
| const end = this.getMark(mark.type.name).expelEnclosingWhitespace && !node?.isText | ||
| ? j : Math.min(j, len); | ||
| marks = marks.slice(0, i).concat(marks.slice(i + 1, end)).concat(mark).concat(marks.slice(j, len)) |
There was a problem hiding this comment.
With Math.min(j, len) the outer mark now stays on the code, and its closing delimiter after the backtick does not close before a letter or **. This replaces a lost mark with literal delimiters.
Input (DefaultSpecsPreset, built; typed in the editor, not produced from markdown): doc(p(a(strong(em('x'))), em(code('y')), strong('z'))).
This branch: [***x***](foo)*`y`***z** → "*", code("y"), "*", strong("z"). 2d79736 and main: [***x***](foo)y**z**, only em on the code is lost. Same with strike: p(a(strong(em(s('x')))), a(s(code('y'))), a('z'))) → link("~~"), code("y"), link("~~z").
The closing problem itself exists on main without reordering: p(em('x'), em(code('y')), strong('z')) → *x`y`***z**. I did not find a local fix: whether the delimiter closes depends on the emitted characters after it, not on the marks of the next node.
Keep line and paragraph separators inside single ending marks. Preserve code-marked leaf nodes without raw line endings. Add regression tests and expected failures for remaining mark boundaries.
|
AI review, revision PR title
Description
Tests
Other
No findings: packages and scripts, architecture. This comment is from AI and may contain inaccuracies. |
Fixes whitespace handling when emphasis, links, and inline code overlap.
*x *becomes*x*.The fixes use the existing serializer spec options and YFM break handling. The changes are limited to the core serializer and its tests.
Adds regression tests and a 45-case research suite. Twelve known serialization issues are marked with
it.fails. These cases still run, and an unexpected pass fails the test run until the marker is removed.Summary by Sourcery
Fix Markdown serialization so whitespace remains lossless and mark boundaries stay correct across overlapping inline marks and line breaks.
Bug Fixes:
Enhancements:
Tests: