fix(serializer): preserve whitespace across overlapping marks by d3m1d0v · Pull Request #1340 · gravity-ui/markdown-editor

d3m1d0v · 2026-10-09T09:00:18Z

Fixes whitespace handling when emphasis, links, and inline code overlap.

Keeps whitespace-only inline code outside reordered marks.
Moves trailing spaces outside ending marks before unmarked hard and soft breaks.
Handles trailing whitespace across all lines when a single escaping mark ends in a block.
Closes emphasis before wholly expelled code whitespace: *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 to preserve whitespace across overlapping marks and line breaks.

Bug Fixes:

Preserve whitespace correctly when emphasis, links, inline code, and other marks overlap during Markdown serialization.
Handle trailing whitespace consistently across multiline content and before marked or unmarked hard and soft breaks.

Enhancements:

Keep whitespace-only inline code outside reordered marks and close enclosing marks before wholly expelled code whitespace.
Use serializer mark options and existing break handling to maintain parseable, round-trippable Markdown output.

Tests:

Add focused regression coverage for overlapping marks, multiline whitespace, custom delimiters, inline code, links, images, headings, and hard or soft breaks.
Add a broader whitespace research suite documenting supported cases and tracking known serialization failures.

gravity-ui · 2026-10-09T09:02:18Z

sourcery-ai

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>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

sourcery-ai · 2026-10-09T09:02:49Z

-                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;


🟡 Medium · 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.

Prompt for AI agents

In `packages/editor/src/core/markdown/MarkdownSerializer.ts` at line 455: **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.

Partially fixed in c4b5fd0, 83aceb3, and 200ace0. Multiline trailing whitespace is now handled for isolated em/strong groups, for groups next to unmarked text, and when renderInline uses fromBlockStart: false. Added regression tests, including headings with U+2028/U+2029 and native link/code guards.

The fallback remains for continuing or overlapping marks. Using the multiline regex in all cases breaks previously correct output, so those remaining cases are not fixed in this PR. The issue is also present on the base branch.

gravity-ui · 2026-10-09T09:05:36Z

makhnatkin · 2026-10-09T11:58:51Z

+            if (
+                !node &&
+                noEsc &&
+                active.every((mark) => this.getMark(mark.type.name).expelEnclosingWhitespace)


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: *![x](a)*\u2028y -> p("*", image, "*\u2028y") // expected: *![x](a)\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.

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.

makhnatkin · 2026-10-09T11:58:55Z


+            // Keep whitespace-only code outside reordered marks.
+            if (reorderedNoEsc && whitespaceOnly) {
+                marks = [];


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.

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.

…reaks

makhnatkin · 2026-10-15T12:53:57Z


+            if (
+                !node &&
+                noEsc &&


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:

Suggested change

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.

makhnatkin · 2026-10-15T12:53:58Z

            }

+            // Keep whitespace-only code outside reordered marks.
+            if (reorderedNoEsc && whitespaceOnly) {


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:

Suggested change

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.

d3m1d0v requested a review from makhnatkin as a code owner October 9, 2026 09:00

sourcery-ai Bot reviewed Oct 9, 2026

View reviewed changes

d3m1d0v force-pushed the codex/serializer-whitespace branch from e2ba70a to 200ace0 Compare October 9, 2026 10:35

makhnatkin reviewed Oct 9, 2026

View reviewed changes

d3m1d0v added 9 commits October 12, 2026 15:06

fix(serializer): preserve code whitespace after mark reordering

8026264

fix(serializer): expel whitespace before unmarked breaks

dba0b3f

fix(serializer): expel trailing whitespace from multiline text

ef17519

fix(serializer): close emphasis before expelled code whitespace

a2a34b9

test(serializer): track whitespace research cases

e5d7ddc

fix(serializer): expel isolated multiline whitespace across marks

6ca6c21

fix(serializer): expel multiline whitespace between unmarked nodes

066c8d9

fix(serializer): expel multiline whitespace in headings

e0d1155

fix(serializer): preserve multiline text and nested emphasis before b…

5ac54a7

…reaks

d3m1d0v force-pushed the codex/serializer-whitespace branch from 200ace0 to 5ac54a7 Compare October 12, 2026 12:07

perf(serializer): avoid quadratic multiline whitespace scanning

535a62d

makhnatkin reviewed Oct 15, 2026

View reviewed changes

Navigation Menu

Repository navigation

fix(serializer): preserve whitespace across overlapping marks - #1340

fix(serializer): preserve whitespace across overlapping marks#1340
d3m1d0v wants to merge 10 commits into
mainfrom
codex/serializer-whitespace

d3m1d0v commented Oct 9, 2026 •

edited by sourcery-ai Bot

Loading

Uh oh!

gravity-ui Bot commented Oct 9, 2026

Uh oh!

sourcery-ai Bot left a comment

Uh oh!

sourcery-ai Bot Oct 9, 2026

Uh oh!

d3m1d0v Oct 9, 2026

Uh oh!

Uh oh!

gravity-ui Bot commented Oct 9, 2026

Uh oh!

makhnatkin Oct 9, 2026

Uh oh!

d3m1d0v Oct 12, 2026

Uh oh!

Uh oh!

makhnatkin Oct 9, 2026

Uh oh!

d3m1d0v Oct 12, 2026

Uh oh!

Uh oh!

makhnatkin Oct 15, 2026

Uh oh!

makhnatkin Oct 15, 2026

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

d3m1d0v commented Oct 9, 2026 • edited by sourcery-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Uh oh!

gravity-ui Bot commented Oct 9, 2026

Uh oh!

sourcery-ai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

sourcery-ai Bot Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

d3m1d0v Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gravity-ui Bot commented Oct 9, 2026

Uh oh!

makhnatkin Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

d3m1d0v Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

makhnatkin Oct 9, 2026

Choose a reason for hiding this comment

Uh oh!

d3m1d0v Oct 12, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

makhnatkin Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

makhnatkin Oct 15, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

d3m1d0v commented Oct 9, 2026 •

edited by sourcery-ai Bot

Loading