Skip to content

markdown: turn a line ending in a code span into a space in the parser - #46402

Open
robobun wants to merge 3 commits into
mainfrom
robobun/ce6956c2/md-code-span-line-endings
Open

robobun wants to merge 3 commits into
mainfrom
robobun/ce6956c2/md-code-span-line-endings

Conversation

@robobun

@robobun robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

Fix

  • The parser hands on the text with a space for each line ending (CommonMark 6.1), still in one text event. HtmlRenderer loses its loop.
  • bun fmt and bun lint (wants_source()) get the text as written: remark's tree keeps the line ending.
  • Verified: test/js/bun/md/ and test/cli/run/markdown-entrypoint.test.ts (53 tests fail without the fix). Also bun fmt conformance, bun lint Markdown tests.
  • Self-reviewed: 10 concerns raised, 10 addressed.

Background

  • The parser reports events (enter_span, text, leave_span) to one RendererImpl per output.
  • Considered a Code arm in each renderer: four copies of the rule, a scan per span each. md4c and cmark convert in the parser.

Downsides

  • A codespan or text callback, or a code component, gets a space where it got a line feed. With heading ids, a setext heading with such a span gets a-b, not ab.
  • A code span costs react(), render() and ansi() 7 more instructions, 33 to 82 in a block of several lines. html() saves 10 or more, or pays up to 14 for a short span there.
Notes

Where it came from. No user reported this and no issue exists. #46069 lists it under "Not in this change".

Where the difference shows

  • ansi() and bun ./file.md: always. The span is printed on two lines, and the second line has no quote bar and no list indent.
  • react(), and a render() that builds HTML: under a style sheet that keeps white space in code (white-space: pre-wrap or break-spaces), and for code that reads the text.
  • Output that keeps white space as written loses a line break that it showed.

Values, before and after. The text that html() writes does not change. Its heading id does: see the last two rows.

input API before after
"`a\nb`" react(), children of code ["a\nb"] ["a b"]
render(), codespan and text callbacks "a\nb" "a b"
ansi(), no colors "a\nb\n" "a b\n"
"> x `a\n> b` y" ansi(), no colors "| x a\nb y\n" "| x a b y\n"
"![`a\nb`](/u)" react(), alt "a\nb" "a b"
"`a\nb`\n===" with headings: { ids: true } or headings: true html(), react(), render() id="ab" id="a-b"
"`a\nb`\n===\n\n## a b\n\n## ab\n", the three ids the same ab, a-b, ab-1 a-b, a-b-1, ab

CRLF and CR inputs give the same values: the parser joins the lines of a block with \n. The last row: a heading beside the span can change its id too, because the count of equal ids moves. Both ids still exist and now lead to another heading.

Design

  • The line feed has one emitter and seven readers: six RendererImpl and HeadingIdTracker. One of the seven converted. Now none has to.
  • Parser::emit_code_text is the only door for Code text. emit_text has a debug_assert against Code.
  • \n is a marked byte with its own bit, MARK_LINE_END. A block on one line does not have the bit in marks_seen, so its spans are not searched. In a longer block, bun_core::strings::index_of_char_usize searches the span once.
  • A span with a line ending is copied into Parser::code_text, which the parse keeps. If the copy cannot be allocated, the call throws an out of memory error, as it does for the output buffers of the renderers.
  • The opt-out is Parser::track, which is RendererImpl::wants_source(). process_table_row uses the same test for \| and for short rows.
  • Set aside: text events per line, as md4c sends them. A span over two lines is then three children in react() and three calls of the text callback.
  • Set aside: the space written into the copy of the block before the walk. The walk reads those bytes again to decide links.

Three forms of the test for a line ending. Instructions per code span, against main. The last column is this PR.

case walk over the parser's marks index_of_char for every span mark bit, then index_of_char
one-line block, render() and ansi(), span of 4 / 11 bytes +9 / +9 +41 / +76 +7 / +7
two-line block, render(), span of 4 / 16 / 256 bytes +31 / +31 / +31 +41 / +58 / +72 +47 / +64 / +78
two-line block, render(), a.b.c d.e.f: default, autolinks, collapseWhitespace, both +31, +67, +40, +76 +76 each +82 each
two-line block, ansi(): a.b.c d.e.f, bun add @types/node@22.1.0, a URL +67, +67, +49 +76, +58, +58 +82, +64, +64
two-line block, ansi(), 256 bytes of a. +1,181 +70 +79
html(), one-line block, span of 4 / 16 bytes -27 / -99 +5 / -50 -29 / -101
html(), two-line block, span of 4 / 16 bytes -6 / -78 +4 / -51 +10 / -45
span of 4 bytes over a line ending, render() +110 +110 +116
  • The walk reads the marks that the parser has for the block and no byte of the span. It costs 9 instructions for each marked byte in the span. ansi() marks ., : and @.
  • The search alone charges every span, in a one-line block too.
  • The third form is in this PR: the least in a one-line block, and a bound in a longer one.

Numbers (release builds of 9b4284f and of c06f5b6, the first commit of this branch on it, x64, BUN_JSC_useJIT=0). The later commit 9e08bd2 changes only the path of a span over a line ending: one try_reserve before the copy.

  • Instructions per paragraph, main -> PR, span code (perf_event_open instructions:u, 200 renders of 1,000 paragraphs, the run with 0 renders subtracted, minimum of 3):
    • no code span: html() 1,615 -> 1,614, render() 1,527 -> 1,527, render() with text 2,382 -> 2,382, ansi() 2,931 -> 2,931.
    • one-line paragraph: html() 2,904 -> 2,875, render() 2,671 -> 2,678, with text 5,348 -> 5,355, ansi() 4,469 -> 4,476.
    • two-line paragraph, no line ending in the span: html() 4,032 -> 4,042, render() 3,703 -> 3,750, with text 7,199 -> 7,246, ansi() 6,056 -> 6,103.
    • span over a line ending: html() 3,667 -> 3,631, render() 3,312 -> 3,428, with text 5,992 -> 6,108, ansi() 5,236 -> 5,354.
    • react() allocates JS objects and its count moves by about 50 between repeats: +6, +98 and +169 in the three cases with a span.
  • html() by the length of the span, PR minus main. One-line block, 1 / 4 / 8 / 16 / 64 / 256 bytes: -10 / -29 / -53 / -101 / -389 / -1,539. Two-line block: +14 / +10 / +6 / -45 / -336 / -1,471.
  • A span over a line ending, by its length, PR minus main. render(), 4 / 20 / 64 / 256 bytes: +116 / +179 / +146 / +178. html(), 4 / 20 / 30 / 40 / 64 bytes: -36 / +453 / +630 / -262 / -2,108. The two rises are one piece of 16 bytes or more where main wrote two shorter pieces: write_html_escaped costs about 500 more from 16 bytes on, on main too.
  • Allocator calls per render of 1,000 paragraphs (hardware breakpoints on the 12 mimalloc entry points that allocate), PR minus main: +0 with no span, with one-line spans and with two-line blocks, in html(), react(), render() and ansi(). +1 per render with 1,000 spans over a line ending.
  • size_of::<Parser>(): 1,296 bytes (-Zprint-type-sizes, release profile), 24 more than main: one Vec<u8>. Bun.markdown.html("x"): 4,038 -> 4,041 instructions per call.
  • Release binary: .text 81,904,485 bytes and .rodata 22,619,084 bytes on both builds (size -A), and the stripped file has the same size. process_inline_content 12,460 -> 12,556 bytes, HtmlRenderer::text 3,292 -> 2,960, the cold copy path 294 (nm -S).
  • Tools: valgrind, perf, strace and bloaty are not installed on the machine.

bench/snippets/markdown.mjs

  • Instructions per call on its three documents, PR against main, as an 8-bit and as a 16-bit string: html() -0.02% to -0.08%, render() +0.01% to +0.04%, ansi() +0.01% to +0.04%. react() is between -1.2% and +0.06%, inside its own spread.
  • Time, mitata, 8 rounds in turn on one core, PR / main of the median p50: the nine Bun rows are 0.997 to 1.059. The marked and remark rows, which never call Bun.markdown, are 0.944 to 1.049.
  • Cycles per call of html() on the large document: 466,498 -> 476,699 (minimum of 9 rounds in turn), at 0.08% fewer instructions. The builds of the three forms above are at 475,037, 492,597 and 476,699, all within 0.13% in instructions. The cycles do not follow the instructions, so a difference of this size comes from the layout of the build.

What does not change

  • html() over the 827 examples of test/js/bun/md/*.txt: 0 differ, with default options and with heading ids.
  • 6,191 Markdown files of this checkout (28.5 MB): html() output is the same for all, with default options and with heading ids. No file has a setext heading with a code span over its line ending. render() and ansi() differ in 71 files. In render() the only bytes that differ are 247 line feeds that are spaces now.
  • bun fmt: the output of --run-prettier-tests (45,692 of 45,778 checks) and of --run-oxfmt-tests is byte-identical on both release builds.
  • bun lint --print-markdown-trees over the 3,308 recorded trees: byte-identical.

Tests

  • md-react.test.ts, md-render-callback.test.ts: a table of spans with the text written out, as a string and as a Buffer. A second table compares a document with the same document on one line: block quote, list item, link, emphasis, image, setext heading, a span between two other spans.
  • md-edge-cases.test.ts: html() rows with exact output, because md-spec.test.ts compares with white space collapsed. A sweep compares the text of every code span of spec.txt in the react() tree with what html() writes. ansi() is compared with the span on one line for five themes.
  • md-edge-cases.test.ts, "bun lint keeps the line ending": the one test for the opt-out. Its tree is what mdast-util-from-markdown 2.0.2 gives. With the track test taken out it fails (alt="a b"), and the bun fmt conformance does not notice. It is not in test/cli/lint/js-parsers.test.ts beside the recorded trees: that test times out under a debug build on main.
  • md-heading-ids.test.ts, markdown-entrypoint.test.ts: one test each.
  • The existing test that folds the react() tree into the render() output stays green.
  • Not taken: a comparison of the whole react() tree with html(). For this fix it finds the same four examples of spec.txt as the sweep.
  • Also run: md-spec.test.ts, gfm-compat.test.ts, test/cli/format/conformance.test.ts, test/cli/lint/js-parsers.test.ts (Markdown), test/cli/lint/explain.test.ts, test/internal/source-lints/, test/integration/bun-types/bun-types.test.ts.

Related pull requests

Not in this change (the same on both builds)

  • The blank before a soft break: "foo \n baz\n" gives <p>foo \nbaz</p> from html(), where example 649 of the spec has <p>foo\nbaz</p>. md-spec.test.ts collapses white space, and Bun.markdown: implement the hardSoftBreaks and collapseWhitespace options #39495 edits those lines.
  • ansi() lines outside their container for an HTML block or text behind a code block: markdown: in ansi(), write a line's quote bars and list indent with its first content #46348.
  • A break in the text of an image: for a soft break html() writes a space and react() and render() a line feed. For a hard break react() writes nothing (alt="ab"). After this change "![`a\nb` c\nd](/u)" gives a b c\nd from react() and a b c d from html().
  • A render() callback can write into its input Buffer. If it writes a line feed into a later code span of the same one-line block, that span hands it on as written, as before.

The parser handed on the text of a code span with the line ending in it,
and only the HTML renderer wrote the space that CommonMark 6.1 asks for.
`Bun.markdown.react()`, `render()` and `ansi()` got the line ending, and
the id of a heading was made from it.

The parser now hands on the text with a space for each line ending, in
one text event as before. A consumer that wants the source (`bun fmt`,
`bun lint`) gets the text as it is written. The HTML renderer loses its
own conversion.
@robobun
robobun requested a review from alii as a code owner October 21, 2026 22:22
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 21fce95 and 9e08bd2.

Files selected for processing: 2
  • src/md/inlines.rs
  • src/md/types.rs

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

Walkthrough

Markdown code spans now render line breaks as spaces under the documented conditions. Parser handling, renderer behavior, documentation, and regression tests reflect this normalization.

Changes

Code-span normalization

Layer / File(s) Summary
Parser and rendering normalization
src/md/parser.rs, src/md/inlines.rs, src/md/html_renderer.rs, src/md/types.rs
The parser marks line endings, and code-span text passes through a helper that replaces newlines with spaces under the documented conditions. The HTML renderer no longer performs newline replacement.
Regression coverage and documented behavior
test/js/bun/md/*, test/cli/run/markdown-entrypoint.test.ts, docs/runtime/markdown.mdx, packages/bun-types/bun.d.ts
Tests cover normalization across input forms and Markdown contexts. Runtime and type documentation state that inline code line breaks are represented as spaces.

Suggested reviewers: jarred-sumner

Priority: Normal

Merge Risk: Minimal · up to 9e08b

No merge-blocking risk is established; the code-span behavior matches the documented contract.

Pre-merge checks: Passed 4
Check name Status Explanation
Description check Passed The description clearly explains the problem, implementation, behavior changes, performance impact, scope, tests, and verification results. It does not use the template headings exactly, but it provid…
Title check Passed The title is concise, specific, and accurately summarizes the primary parser change for line endings inside code spans.
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.
Full details: Description check

Explanation

The description clearly explains the problem, implementation, behavior changes, performance impact, scope, tests, and verification results. It does not use the template headings exactly, but it provides the required information in equivalent sections.

  • 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

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 11:30 AM PT - Oct 27th, 2026

✅ @robobun, your commit 9e08bd2ac38743f3146a9c1891ce0581cb2c51c7 passed in Build #133788! 🎉


🧪   To try this PR locally:

bunx bun-pr 46402

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

bun-46402 --bun

@robobun

robobun commented Oct 21, 2026 •

Copy link
Copy Markdown
Collaborator Author

Status: ready for a maintainer review. The diff is green on CI so far. In build 133788 for 9e08bd2, 179 of 181 jobs passed and none failed. Two macOS x64 test jobs still wait for a machine.

Three tests failed once in that build and passed when they ran again: spawn_waiter_thread.test.ts, test-http-agent-keepalive.js and nth-anplusb-ident.test.ts. None of them is a Markdown test.

The branch has main (9b8347a) merged in, with no conflict. The last commit, 9e08bd2, takes the two suggestions of the review.

How I reproduced the problem on a release build of main (9b4284f):

const text = "`a\nb`";
const log = (name, value) => console.log(name, JSON.stringify(value));
log("html", Bun.markdown.html(text));
log("react", Bun.markdown.react(text).props.children[0].props.children[0].props.children);
log("render", Bun.markdown.render(text, { codespan: code => "[" + code + "]" }));
log("ansi", Bun.markdown.ansi("> x `a\n> b` y", { colors: false }));
  • main: html "<p><code>a b</code></p>\n", react ["a\nb"], render "[a\nb]", ansi "| x a\nb y\n"
  • this branch: html "<p><code>a b</code></p>\n", react ["a b"], render "[a b]", ansi "| x a b y\n"

The five changed test files have 53 tests that fail on main and pass on this branch. The same holds after the merge. A debug build with the src/ of main 9b8347a fails 53 of the 405 tests. A debug build of this branch passes all 405.

@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.

Comment thread src/md/inlines.rs Resolved
Comment thread src/md/inlines.rs Resolved
… memory error

`emit_code_text` takes no text type. Only a code span emits through it,
so the `debug_assert` in `emit_text` and the doc of `RendererImpl::text`
name `Code` alone.

The copy of a span with a line ending reserves its space fallibly and
returns `ParserError::OutOfMemory`, as the output buffers of the
renderers do.

@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.

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.

1 participant