Fix markdown preview diff collapsing list items - #335618
Fix markdown preview diff collapsing list items#335618Birarpanjot Singh Kanwer (arpankanwer) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
List-marker detection still mishandles code-block contexts.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes Markdown preview diffs collapsing list items by preserving list-marker syntax during diff highlighting.
Changes:
- Detects ordered and unordered list prefixes.
- Moves injected diff markers after list prefixes.
File summaries
| File | Summary |
|---|---|
extensions/markdown-language-features/src/preview/documentRenderer.ts |
Preserves list parsing during diff rendering. Moderate findings remain regarding list detection in indented and fenced code blocks (2 votes and 1 vote). |
Review details
Suppressed comments (2)
extensions/markdown-language-features/src/preview/documentRenderer.ts:282
- This regex also treats list-looking text inside fenced code blocks (for example, a code line
- foo) as a list marker. If the diff changes that-prefix, the new code skips the inner highlight even though it is code content, so the preview loses part of the diff. The marker check needs Markdown block context rather than matching every line with this shape.
const match = /^[ \t]*(?:[-*+]|\d+[.)])[ \t]+/.exec(line);
extensions/markdown-language-features/src/preview/documentRenderer.ts:282
- This only recognizes a list marker after indentation at the start of the line and only when it has following whitespace. Valid Markdown list items can also be inside blockquotes (for example,
> - item, including nested blockquotes) or be empty (-/1.at end of line). For a diff spanning those prefixes, this returns0, so the marker spans are still injected before markdown-it parses the line and the list can collapse again. Match the complete blockquote/list prefix and allow an end-of-line marker.
const match = /^[ \t]*(?:[-*+]|\d+[.)])[ \t]+/.exec(line);
- Files reviewed: 1/1 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| /** Length of the leading list marker (`- `, `1. `) that must stay intact for list parsing. */ | ||
| function getListMarkerPrefixLength(line: string): number { | ||
| const match = /^[ \t]*(?:[-*+]|\d+[.)])[ \t]+/.exec(line); |
There was a problem hiding this comment.
Same tradeoff as noted below — skipping the highlight keeps code rendering cleanly; fixing code-vs-list properly needs block context, out of scope here.
|
Addressed in 0aa93ee: the prefix now covers blockquote markers and end-of-line markers, verified with render checks. Leaving the fenced-code concern as is — the old code injected raw spans there too (which markdown-it escapes into literal text), so skipping the highlight renders code cleanly; block-aware detection deserves its own scope if you want it. |
Fixes #335469.
Markdown preview diffs collapsed changed list items onto one line with literal diff markers visible. This keeps the injected inner-change highlight spans out of the list marker prefix (
-,1., `) so the line still parses as a list item.Root cause: inline diff spans injected before markdown-it parse split the list marker, so the list no longer parsed and items rendered inline.
Verification: tsc clean, plus dev-window side-by-side check with before/after screenshots.
Screenshots:
Before:

After:
