Skip to content

fix(core): preserve multiline text and formatting in serializer - #1391

Open
d3m1d0v wants to merge 1 commit into
mainfrom
codex/fix-serializer-multiline-whitespace
Open

d3m1d0v wants to merge 1 commit into
mainfrom
codex/fix-serializer-multiline-whitespace

Conversation

@d3m1d0v

@d3m1d0v d3m1d0v commented Oct 30, 2026 •

Copy link
Copy Markdown
Member

Summary

Fix text loss and broken formatting when the serializer handles multiline text, spaces, and line breaks. This continues the whitespace work from #1388.

  • Preserve text after LF/CR/CRLF and U+2028/U+2029 inside nested formatting and links.
  • Close formatting before trailing spaces and unmarked breaks. Keep soft breaks from becoming hard breaks after two spaces.
  • Replace raw line endings in headings with spaces, including inline code. Preserve images next to reordered code marks.
  • Avoid repeated lookahead over the same whitespace chain while keeping the current serializer structure and public API.

Tests and remaining scope

Integration tests cover text and formatting after serialization and parsing, link targets, images, and cached heading output without changes to the source document. Fixed cases are converted from expected-failure tests to regular tests.

Expected-failure tests remain for known code-space, code/link, and Unicode delimiter problems, including the deferred multiline code/link cases. This PR does not resolve all serializer boundary issues.

Summary by Sourcery

Preserve multiline text and formatting boundaries across Markdown serialization and parsing.

Bug Fixes:

  • Preserve multiline text, whitespace, formatting marks, links, images, and line breaks during Markdown serialization and round-trip parsing.
  • Normalize raw line endings in headings and inline code without losing surrounding text or cached serializer output.
  • Prevent trailing whitespace and soft breaks from changing formatting boundaries or being misinterpreted as hard breaks.

Enhancements:

  • Reduce redundant whitespace lookahead while retaining the existing serializer structure and public API.

Tests:

  • Expand integration coverage for multiline text, Unicode separators, whitespace boundaries, links, images, headings, inline code, and hard and soft breaks.
  • Promote fixed expected-failure cases to regular passing tests while retaining coverage for unresolved serializer boundary issues.

@gravity-ui

gravity-ui Bot commented Oct 30, 2026

Copy link
Copy Markdown

Storybook Deployed

@gravity-ui

gravity-ui Bot commented Oct 30, 2026

Copy link
Copy Markdown

🎭 Playwright Report

@d3m1d0v
d3m1d0v marked this pull request as ready for review October 30, 2026 17:23
@d3m1d0v
d3m1d0v requested a review from makhnatkin as a code owner October 30, 2026 17:23
if (this.out.endsWith(' ')) {
let end = this.out.length;
while (/[ \t]/.test(this.out.charAt(end - 1))) --end;
this.out = this.out.slice(0, end);

@makhnatkin makhnatkin Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated
"Generated" means mostly the model, "Edited" means mostly me.

[P1] Keep the previous block's output when handling soft_break

When a paragraph starts with soft_break, the previous block has not been closed by flushClose() yet, so this code trims that block's trailing spaces. renderTopLevelContent() then caches the new block as this.out.slice(outBefore.length) with the old length and drops the beginning of its output.

Reproduced with DefaultSpecsPreset:

const first = p('x      ');
const second = p(sb(), 'yz');
serializer.serialize(doc(first, second)); // "x\n\n\nyz"
serializer.serialize(doc(first, second, p('new paragraph')));
// "x      \n\nnew paragraph": the text "yz" is lost

With two trailing spaces, p('x '), the second serialization returns x \ny, which parses as a single paragraph with a hard_break.

The base version keeps yz. Trimming should be limited to the current block; please add a test that serializes twice with reused nodes.

Comment on lines +398 to +400
if (keepLinkedWhitespace) {
marks = marks.filter(mark => this.getMark(mark.type.name).expelEnclosingWhitespace)
.concat(marks.filter(mark => !this.getMark(mark.type.name).expelEnclosingWhitespace));

@makhnatkin makhnatkin Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated
"Generated" means mostly the model, "Edited" means mostly me.

[P2] Take the preceding character into account when moving formatting outside a link

The reordering produces an invalid opening delimiter when a letter precedes the link. With DefaultSpecsPreset, doc(p('A', a(em('B ')))) now serializes to A*[B ](foo)*: after parsing, B loses emphasis and both asterisks become literal text. The base version outputs A[*B*](foo) and keeps the emphasis. strong and strike behave the same way.

Before moving delimiters outside the link, check that they remain valid at the new position; please add a case with a link directly after plain text.

Comment on lines +490 to +492
if (node?.type.name === 'soft_break') {
leading = leading.replace(/[ \t]+$/, '');
if (this.out.endsWith(' ')) {

@makhnatkin makhnatkin Oct 30, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated
"Generated" means mostly the model, "Edited" means mostly me.

[P2] Keep spaces inside a link that is being closed

This trimming runs before active marks are closed and removes link content: a document parsed from [x ](foo)\ny serializes back as [x](foo)\ny. The base version keeps both spaces. They cannot produce a hard break, since ](foo) is written before the newline.

Trimming should take closing delimiters into account; please add a round-trip check for a link with two trailing spaces before a soft break.

@makhnatkin makhnatkin left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI-generated
"Generated" means mostly the model, "Edited" means mostly me.

Changes requested: repeated serialization can drop a whole paragraph. Loss of link formatting and removal of spaces inside links are also confirmed. Reproductions are in the three inline comments.

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've reviewed your changes and they look great!


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

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 1 issue

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="492-495" />
<code_context>
+
+            if (node?.type.name === 'soft_break') {
+                leading = leading.replace(/[ \t]+$/, '');
+                if (this.out.endsWith('  ')) {
+                    let end = this.out.length;
+                    while (/[ \t]/.test(this.out.charAt(end - 1))) --end;
+                    this.out = this.out.slice(0, end);
+                }
+            }
</code_context>
<issue_to_address>
**Soft break deletes preceding block output**

When a block ends in spaces and the next block starts with a soft break, the soft-break handler trims `this.out` across the top-level block boundary. `renderTopLevelContent` then slices the current block’s cache entry from the old output length, so it omits the beginning of that block; a later serialization with the same preceding block type reuses the truncated output.

Limit soft-break trimming to spaces emitted within the current inline block, rather than trimming the shared output buffer across block boundaries.
</issue_to_address>

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

Comment on lines +492 to +495
if (this.out.endsWith(' ')) {
let end = this.out.length;
while (/[ \t]/.test(this.out.charAt(end - 1))) --end;
this.out = this.out.slice(0, end);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Medium · Soft break deletes preceding block output

When a block ends in spaces and the next block starts with a soft break, the soft-break handler trims this.out across the top-level block boundary. renderTopLevelContent then slices the current block’s cache entry from the old output length, so it omits the beginning of that block; a later serialization with the same preceding block type reuses the truncated output.

Limit soft-break trimming to spaces emitted within the current inline block, rather than trimming the shared output buffer across block boundaries.

Prompt for AI agents
In `packages/editor/src/core/markdown/MarkdownSerializer.ts` at lines 492-495:

**Soft break deletes preceding block output**

When a block ends in spaces and the next block starts with a soft break, the soft-break handler trims `this.out` across the top-level block boundary. `renderTopLevelContent` then slices the current block’s cache entry from the old output length, so it omits the beginning of that block; a later serialization with the same preceding block type reuses the truncated output.

Limit soft-break trimming to spaces emitted within the current inline block, rather than trimming the shared output buffer across block boundaries.

@makhnatkin

Copy link
Copy Markdown
Collaborator

AI review, revision ea8cc0904e

Tests

Architecture

  • The core serializer now checks node names owned by extensions: 'heading' (headingNodeName in extensions/markdown/Heading/HeadingSpecs/const.ts) and 'soft_break' (BreakNodeName.SoftBreak in extensions/markdown/Breaks/BreaksSpecs/index.ts). Before this PR, renderInline detected breaks only through type.spec.isBreak. If the heading node is renamed, or a schema uses another heading node, line-ending normalization silently stops working. Line-ending normalization in headings belongs in headingToMarkdown (extensions/markdown/Heading/HeadingSpecs/utils.ts), before the state.renderInline(node, …) call. That placement also lets the lookahead in renderInline see the normalized siblings. — MarkdownSerializer.ts:370, MarkdownSerializer.ts:490

Other

  • Blocker: the three earlier inline comments are still open, and no test covers them. The soft-break trimming in the earlier comment on MarkdownSerializer.ts:495 still trims this.out across the block boundary. As a result, the top-level cache stores truncated output. The link reordering in the earlier comment on MarkdownSerializer.ts:400 and the space trimming in the earlier comment on MarkdownSerializer.ts:492 are unchanged. The requested tests are still missing: repeated serialization with reused nodes after a leading soft_break, a link directly after plain text, and a link with two trailing spaces before a soft break. — MarkdownSerializer.ts:490

No findings: PR title, description, packages and scripts.

This comment is from AI and may contain inaccuracies.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants