Skip to content

markdown: decide in one place whether a blank line makes a list loose - #46119

Open
robobun wants to merge 3 commits into
mainfrom
robobun/059f1f5a/md-loose-list-blank-line
Open

robobun wants to merge 3 commits into
mainfrom
robobun/059f1f5a/md-loose-list-blank-line

Conversation

@robobun

@robobun robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Bun.markdown.html("- code\n\n text\n") gives </pre>\ntext</li>: no <p>. The same happens behind HTML of type 6 or 7, and before a marker-only line. CommonMark 5.3 makes these lists loose.
  • The reverse: - > - b\n >\n- c\n becomes loose, but its >-only line is in the quote.
  • Found by fuzzing, no user report. - a\n\n- [ ]\n b\n regressed in 1.4.4 with Add bun lint and bun format #44805.

Fix

  • analyze_line kept a one-line flag that three paths dropped (src/md/blocks.rs:224, :248, :792). Now an empty line in a list item stores one past the innermost > on it. The next non-empty line in a deeper container makes its list loose, unless it continues indented code.
  • A blank line that ends HTML types 6/7 takes the blank-line arm.
  • Verified: test/js/bun/md/md-edge-cases.test.ts (69 new tests, 51 fail on main). Also test/js/bun/md/, test/cli/format/conformance.test.ts.
  • Self-reviewed: 18 concerns raised, 16 addressed. Not done: an md4c report, a bun lint differential test (Notes).

Background

  • A loose list wraps item paragraphs in <p>. The flag comes from md4c, which has the same paths.
  • Considered patching the three sites: that needs the same quote guard and 11 to 17 more instructions per line.

Downsides

Notes

Where it comes from. A differential fuzz run reported five forms in the team chat. There is no GitHub issue and no user report. Of 11,888 Markdown files on the build machine (5,579 with a list), none renders differently in HTML or ANSI between release builds of main and of this branch. The wrong output is a missing or an extra <p>, never lost text.

What changes (Bun.markdown.html, main and this branch):

input main this branch
- code\n\n text\n ...</pre>\ntext</li> ...</pre>\n<p>text</p>\n</li>
- <div>\n\n text\n <div>\ntext</li> <div>\n<p>text</p>\n</li>
- code\n\n- text\n <li>text</li> <li>\n<p>text</p>\n</li>
- a\n\n-\n <li>a</li> <li>\n<p>a</p>\n</li>
- a\n\n >\n <li>a\n<blockquote> <li>\n<p>a</p>\n<blockquote>
- > - b\n >\n- c\n <li>\n<p>c</p>\n</li> <li>c</li>

render(), react(), ansi(), bun file.md and an imported .md file use the same parser. The first three have a test. The last two call the same two functions and have none.

The two shapes from #44805 (c39c718, first released in 1.4.4: tag bun-v1.4.4 has it, bun-v1.4.3 does not). Checked on 1.3.13, 1.4.2, 1.4.4, main and this branch.

  • A blank line, then an item with only a task mark: - a\n\n- [ ]\n b\n is loose on 1.3.13 and 1.4.2, tight on 1.4.4 and main, loose here. Add bun lint and bun format #44805 types that line Blank, and the old reader skipped Blank lines.
  • A blank line behind an HTML line that follows a table in an item (- | a |\n |---|\n | 1 |\n </details>\n\n text\n): <p>text</p> on 1.4.2, bare text on 1.4.4 and main, <p>text</p> here.
  • This is not the 1.4.2 output byte for byte. Two differences from Add bun lint and bun format #44805 stay: an item with only a task mark has no empty <p></p>, and - [ ]\n\n text\n is loose.

bun fmt shows it inside one binary. On main bun fmt rewrites - a\n\n-\n b\n to - a\n\n- b, and the two render with 0 and 2 <p>. It rewrites - > - b\n >\n- c\n to - > - b\n- c, with 1 and 0 <p>. On this branch both spellings of each render the same. This is a repro for the marker and quote shapes only, not a claim that the formatter and the renderer now agree everywhere.

How it works now.

  • State: blank_line_in_list, one u32. It is NO_BLANK_LINE, or the lowest index in containers of a list that the empty line before this one can make loose. blank_line_marks is how many containers a blank line has the marker or the indentation of. The blank line writes it before the lazy match, so the setter does not scan the lists that were matched lazily.
  • Setter: an empty line stores NO_BLANK_LINE if it is in no list item. Otherwise it stores one past the innermost > that is on the line. In a run of empty lines the last one decides.
  • Reader: the next line that is not empty takes the value. It does nothing if the line is more of the indented code that the empty line is in, or if the line is in no container above the stored index. Otherwise it marks the innermost list that the line is in, or is the next item of.
  • A line with only a marker is not empty, so it reads. That was the third path.
  • A blank line in a fenced block or in HTML of types 1 to 5 is content. It neither sets nor reads, as before. cmark and commonmark.js keep - ```\n a\n\n- b\n tight, and so does this.

md4c. By a read of mity/md4c@60e8065 (not a run), src/md4c.c has the same four paths: :6743, :6722-6726, :7063, and :6671-6676 with :6747-6749. No md4c issue covers them, and I did not file one. From here analyze_line does not mirror md_analyze_line for looseness: a later diff of the port against upstream must not bring the flag back.

Measurements. Release builds (bun run build:release, linux-x64) of 91e7e3b and of f107088, which is this change on that commit.

Instructions per input line in the block pass. gdb single-step count of one Bun.markdown.html call, documents of 100 and 200 lines, the difference divided by 100. Symbols: Parser::process_doc, analyze_line, is_container_mark, is_hr_line, the two HTML block conditions, is_table_underline, count_table_row_columns, leave_child_containers, end_current_block, and the two new functions.

input, repeated main this branch
word word 459.1 425.1
p, blank line 498.7 458.1
# h 756.1 722.1
- item 1402.2 1370.1
a line in a list item 499.1 467.1
> word 517.1 483.1
> - a 1455.1 1421.1
a line in a fence 324.1 287.1
- a, - b, blank, text, blank 938.5 905.3
- a, blank 898.6 878.6
- c, blank 850.6 841.1
> - a, > 948.1 929.1
  • Most of the drop is one thing. In this build LLVM inlines analyze_line into the per-line loop of process_doc. On main it is a function of 15,034 bytes with a call per line. The numbers are instruction counts of one link, not a property of the design.
  • Looseness code on a line that is not empty: main runs 6 instructions with 3 stores (load and spill of the flag, two zero stores, test and branch). This branch runs 4 with no store (cmpb, jne, cmpl, je). The reader is one call in a block out of line.
  • One whole Bun.markdown.html call, every instruction of the binary: the small document of bench/snippets/markdown.mjs 21,648 to 21,382, the medium one 115,620 to 113,780.
  • Code bytes (nm -S, size): process_doc 8,822 + analyze_line 15,034 = 23,856 on main. process_doc 23,942 + blank_line_in 117 + end_blank_line_in_list 69 = 24,128 here. .text 107,072,443 to 107,072,699. The stripped bun has 107,243,232 bytes on both.
  • Struct bytes (rustc -Zprint-type-sizes): Parser 1,272 to 1,280, one per call. Container 32, BlockHeader 16, Line 20, all unchanged.
  • Allocations during one parse (gdb breakpoint counts of mi_malloc and mi_realloc): 5 + 17 for 200 lines of a loose list and 5 + 19 for 200 items, on both. Block metadata (smallest setMaxMarkdownBlockBytesForTesting limit that parses a fixed document): the same on both for three documents.
  • '- '.repeat(n) + 'a\n' + '\n'.repeat(n), CPU ms: n = 10,000 43.7 to 42.1, n = 20,000 172.6 to 165.3, n = 40,000 720.8 to 696.7. Quadratic before and after.
  • Wall clock, Bun.markdown.html on the three documents of bench/snippets/markdown.mjs, 28 interleaved runs per build on one core: small 1,854 to 1,869 ns, medium 8,340 to 8,330 ns, large 139,052 to 139,739 ns. A second run of the same pair gave +1.6%, +0.6% and +1.7%. Two builds of main five commits apart differ by 2.0% to 4.1% on the same documents, so this is inside the noise. The bench file itself needs marked and remark, which are not installed there, so the documents were timed with a loop.

What the first version cost. The first version put is_empty in the tail as one expression and kept two more locals alive across the function. It cost 22 to 39 instructions more per line than main on the rows above. The cause was the register allocation of the whole function, not the looseness code. The final version keeps no new local alive across the function, and the two rare paths are out of line, after the precedent of is_html_block_start_condition.

The patch of the three sites. A draft with the quote guard is +25/-3 in the two files, against +64/-39 here. Without the guard it makes quote inputs wrong that main has right, such as - a\n- > - code\n >\n- c\n (three such rows are in the test). Built in a small harness that links the same bun_md bitcode, it runs 11 to 17 instructions more per line than main, where this branch runs 22 to 40 fewer in the same harness. It leaves six writes of the flag in the line-typing loop. That shape is how #44805 made - a\n\n- [ ]\n b\n tight.

Where the expected values come from.

  • commonmark.js 0.31.2, markdown-it 14.3.3 and micromark 4.0.3 were run once on 43 inputs. They agree on every form that gains <p>.
  • cmark and md4c were read, not run.
  • The rows that lose <p> follow CommonMark 5.3 (the example with > b and > in an item) and a read of cmark's S_ends_with_blank_line, which does not look into a block quote.
  • A reference definition behind the blank line, as the last block of the list (- a\n- code\n\n [r]: /u\n), goes from tight to loose. commonmark.js, markdown-it and micromark are loose there. cmark is tight by a read. Bun was already loose when the block before the blank line is a paragraph.
  • Constructed lists that CommonMark 5.3 decides: 306 of 1,444 wrong on main, 0 here. 100,000 random list documents: 574 gain <p>, 7 lose it, nothing else differs.

Left as it is.

  • - <pre>\n\n- b\n: a blank line in unclosed HTML of types 1 to 5, then a sibling. commonmark.js 0.31.2 is tight: the blank line is a line of the HTML block, and endsWithBlankLine compares source positions (lib/blocks.js:78-81). cmark 0.31.2 is loose by a read: add_text_to_container excludes fenced code but not HTML (src/blocks.c:1175-1180). Stays tight, and the test row says that the two disagree.
  • The ANSI renderer writes text that follows a block in a tight item at column 0, and without the bars of the block quotes around the item. It does so on 1.3.13, 1.4.2 and 1.4.4 too: > - a\n> > b\n> >\n> c\n prints c alone at column 0 there. This PR moves the newly tight quote shape onto that path and the newly loose shapes off it (- code\n\n text\n has text at column 0 on 1.4.4 and at column 4 here).
  • The quote shape, Bun.markdown.ansi("> - a\n> > - b\n> >\n> c\n", { colors: false }): "| * a\n|\n| | * b\n|\n| c\n" on 1.4.4, "| * a\n|\n| | * b\nc\n" here.
  • The renderer fix is markdown: in ansi(), write a line's quote bars and list indent with its first content #46348. A local build of this branch with markdown: in ansi(), write a line's quote bars and list indent with its first content #46348 merged in gives "| * a\n|\n| | * b\n| c\n". The tests of both pass together: 1,403 in test/js/bun/md/ and test/cli/run/markdown-entrypoint.test.ts.
  • bun fmt and bun lint decide looseness by themselves from line positions (src/format/markdown/parse.rs). A test that compares that tree with Bun.markdown can only pass once this has landed.
  • A blank line still walks every open list in the lazy match. That is the quadratic case above.

Not done from the self-review.

  • A report to md4c. The four paths are listed above. A maintainer should decide whether Bun files it.
  • A differential test between the tree of bun lint and Bun.markdown. It fails on main, so it can only follow this change. The formatter follows Prettier's rule for each item on purpose, so the two need not agree on every input.

Suites run on the debug build. With main at 2d6d445 merged in: test/js/bun/md/ (1,266 pass), test/cli/format/conformance.test.ts, test/cli/run/markdown-entrypoint.test.ts. Before that merge: test/internal/source-lints/. The 27 rows of #45924 pass with and without the change.

A list is loose if a blank line is between two of its items, or between
two blocks of one item (CommonMark 5.3). The parser kept that in a flag
for one line, and three paths dropped it: a blank line behind indented
code, a blank line that ends an HTML block of type 6 or 7, and a blank
line before a line that has only a container marker. In the other
direction, a line with only the `>` of a quote that holds a list made
the list around the quote loose.

The end of `analyze_line` now has one setter and one reader. An empty
line in a list item records the lowest container index that it can
make loose: one past the innermost `>` on that line. The next line that
is not empty reads it, and makes its list loose unless it is more of
the indented code that the empty line is in. The blank line that ends
HTML of types 6 and 7 falls through to the one blank-line arm, so that
arm is the only producer of `is_empty`.

No statement about looseness is left in the line-typing loop, and the
two rare paths are out of line.
@robobun

robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 9:54 AM PT - Oct 21st, 2026

✅ @robobun, your commit d2e069fd29c1a17c8c6c3b41ae05a001be81f2ab passed in Build #130084! 🎉


🧪   To try this PR locally:

bunx bun-pr 46119

That installs a local version of the PR into your bun-46119 executable, so you can run:

bun-46119 --bun

@robobun

robobun commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status

Reproduction: Bun.markdown.html("- code\n\n text\n") returns <ul>\n<li>\n<pre><code>code\n</code></pre>\ntext</li>\n</ul>\n on 1.3.13, 1.4.2, 1.4.4 and main (91e7e3b). The paragraph has no <p>. The loose and tight lists group in test/js/bun/md/md-edge-cases.test.ts has 69 tests. On 1.4.4 and on a debug build of main 51 of them fail. On this branch all pass.

PR: #46119

@coderabbitai

coderabbitai Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Walkthrough

The parser now determines list looseness from blank-line boundaries within list containers. HTML block continuation handling also changed. Edge-case tests cover list output across block types and rendering APIs.

Changes

Markdown list parsing

Layer / File(s) Summary
Parser blank-line state
src/md/parser.rs
Parser replaces the previous list-loosening flag with blank-line container tracking and a sentinel for cases without a qualifying blank line.
List looseness and HTML continuation
src/md/blocks.rs
Block parsing uses blank-line boundaries to mark matching list containers loose, preserves indented-code continuation behavior, and updates HTML block end-condition handling.
List behavior validation
test/js/bun/md/md-edge-cases.test.ts
Tests cover blank-line placement, nested structures, block types, options, line endings, rendering APIs, and generated list and block combinations.

Suggested reviewers: jarred-sumner

Merge Risk: Minimal · up to d2e06

No actionable issue is established from the available evidence; the change is mergeable after normal checks.

Pre-merge checks: Passed 4

Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary change: centralizing the decision for whether a blank line makes a list loose.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description explains the problem, the fix, and how the author verified the changes. It does not use the template headings, but it provides the required information and substantial supporting conte…
Full details: Description check

Explanation

The description explains the problem, the fix, and how the author verified the changes. It does not use the template headings, but it provides the required information and substantial supporting context.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

ℹ️ Recent review info

⚙️ Run configuration

  • Configuration used: Repository: oven-sh/bun/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: be1f2655-ca0f-4ffa-b581-00ed31705d3f
📥 Commits

Reviewing files that changed from the base of the PR and between f107088 and d2e069f.

📒 Files selected for processing: 1

  • test/js/bun/md/md-edge-cases.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline findings, I also checked the release-build safety of the new reader: blank_line_in only runs when n_parents > 0, and end_blank_line_in_list is reached with count = n_parents + n_brothers <= n_containers, so the count - 1 index is in bounds; the debug_assert! on > holds because any quote the blank line did not mark is closed before the next line and any quote it did mark lies below floor. The closing-fence and fenced/HTML-1-5 content paths neither set nor read blank_line_in_list, matching the old flag's clearing there.

Extended reasoning...

The change replaces a per-line looseness flag in the md4c-port block parser (src/md/blocks.rs, src/md/parser.rs) with two Parser fields and a setter/reader pair, and restructures the HTML type 6/7 end-condition branch; it touches no security-sensitive surface beyond parsing untrusted Markdown with safe indexing. Two confirmed findings are posted inline, so a human look is already signalled; this note only records the index-bounds, debug_assert, and fence/HTML-content paths that were examined and ruled out.

Comment thread src/md/blocks.rs Resolved
Comment thread src/md/blocks.rs Resolved

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code review found no issues

No high-confidence issues detected in this change.

@robobun

robobun commented Nov 9, 2026

Copy link
Copy Markdown
Collaborator Author

A second report of this bug arrived. Its input has two items, and the first item is only an indented code block:

Bun.markdown.html("1.     code\n\n1. text\n");
  • Bun 1.4.3 gives <li>text</li> for the second item. commonmark.js 0.31.2, markdown-it 15.0.3 and micromark 4.0.3 give <li><p>text</p></li>.
  • This branch fixes it. I merged d2e069f onto main 2596ac5 (no conflict) and built it. The input above and - code\n\n- text\n both give <p>text</p> on that build.
  • On that build, test/js/bun/md/ passes (1,266 tests). On Bun 1.4.3, 50 of the 69 tests in the loose and tight lists group fail.
  • I compared 27 list inputs (indented code, fenced code, blank lines between items and between blocks) with commonmark.js. On Bun 1.4.3, 14 differ. On that build, 0 differ.

No new test is necessary. The two items row of the every kind of block test already builds this input as 1. x\n\n1. M\n.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants