Skip to content

Fix #3100: Hoist whitespace outside markdown emphasis markers#3343

Open
JiuqingSong wants to merge 2 commits into
masterfrom
fix-3100-markdown-whitespace
Open

Fix #3100: Hoist whitespace outside markdown emphasis markers#3343
JiuqingSong wants to merge 2 commits into
masterfrom
fix-3100-markdown-whitespace

Conversation

@JiuqingSong
Copy link
Copy Markdown
Collaborator

Summary

  • Fixes convertContentModelToMarkdown() may generate invalid markdown #3100. The markdown emitter wrapped segment text with **/*/~~ verbatim, so a text segment with surrounding whitespace produced invalid CommonMark (e.g. **world **how instead of **world** how).
  • textProcessor in createMarkdownParagraph.ts now hoists leading/trailing whitespace outside the emphasis markers and skips wrapping for whitespace-only segments.
  • One existing test in createMarkdownBlockgroupTest was asserting the buggy > *text *[link](...) output — updated to the correct > *text* [link](...).

Test plan

  • yarn test:fast --testPathPattern=roosterjs-content-model-markdown — 199/199 pass
  • yarn eslint — clean
  • Five new unit tests cover: trailing whitespace inside bold, leading whitespace inside italic, surrounding whitespace inside strikethrough, combined bold+italic, and whitespace-only segments.

🤖 Generated with Claude Code

JiuqingSong and others added 2 commits May 18, 2026 13:00
The markdown emitter wrapped segment text with **/*/~~ verbatim, so a
text segment ending or starting with whitespace produced invalid
markdown (e.g. "**world **how" instead of "**world** how"). Move
leading/trailing whitespace outside the emphasis markers before
wrapping, and skip wrapping for whitespace-only segments.

An existing test in createMarkdownBlockgroupTest was asserting the
buggy "*text *[link](...)" output; updated to the correct
"*text* [link](...)".

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes issue #3100 where the markdown emitter could produce invalid CommonMark by wrapping segments that contain leading or trailing whitespace inside emphasis markers (e.g. **world **how). The textProcessor in createMarkdownParagraph now hoists surrounding whitespace outside the **/*/~~ markers and returns whitespace-only segments untouched. An existing test that asserted the buggy output was updated, and five new unit tests cover the fix.

Changes:

  • Refactor textProcessor to extract leading/trailing whitespace via regex and apply emphasis markers only around the non-whitespace core, including when a link is present.
  • Update the previously incorrect > *text *[link](...) assertion to > *text* [link](...).
  • Add unit tests for bold trailing whitespace, italic leading whitespace, strikethrough surrounding whitespace, combined bold+italic, and whitespace-only segments.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/roosterjs-content-model-markdown/lib/modelToMarkdown/creators/createMarkdownParagraph.ts Reworked textProcessor to hoist whitespace outside emphasis markers and skip wrapping whitespace-only segments.
packages/roosterjs-content-model-markdown/test/modelToMarkdown/convertContentModelToMarkdownTest.ts Added five regression tests covering the new whitespace-hoisting behavior.
packages/roosterjs-content-model-markdown/test/modelToMarkdown/creators/createMarkdownBlockgroupTest.ts Updated an existing assertion that encoded the buggy markdown output.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const [, leading, core, trailing] = match ? match : ['', '', text.text, ''];

if (!core) {
return text.text;
Comment on lines +60 to +61
const match = /^(\s*)([\s\S]*?)(\s*)$/.exec(text.text);
const [, leading, core, trailing] = match ? match : ['', '', text.text, ''];
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.

convertContentModelToMarkdown() may generate invalid markdown

2 participants