Repository navigation
Conversation
…ts first content A line break in inline content wrote the indent of the next line at once, also when no line followed. An HTML block in a list item or a block quote ended with that indent and started without one, so the next list marker moved right. Text behind a code block in a tight list item was at column 0. A line break now writes the indent of the next line only when more of the same text follows. Each inline write that can be the first of a line writes the indent when nothing is on the line yet. A line that has only its indent is ended like a line with content.
|
Updated 5:05 PM PT - Oct 21st, 2026
❌ @robobun, your commit 1c7a876 has some failures in 🧪 To try this PR locally: bunx bun-pr 46348That installs a local version of the PR into your bun-46348 --bun |
|
Reproduction (bun 1.4.3 and main): const plain = md => JSON.stringify(Bun.markdown.ansi(md, { colors: false }));
console.log(plain("- a\n <div>\n- b\n"));
// " * a\n<div>\n * b\n"
// `<div>` is at column 0 and the second marker is at column 6.
console.log(plain("- a\n ```\n x\n ```\n b\n- next\n"));
// " * a\n\n +-\n | x\n +-\nb\n * next\n"
// `b` is at column 0.The same happens with PR: #46348 |
…nsi-container-prefix
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
WalkthroughThe ANSI Markdown renderer centralizes line-start indentation and line breaking. CLI and JavaScript tests check indentation across HTML blocks, nested containers, wrapping, links, and Kitty images. ChangesANSI Markdown line indentation
Suggested reviewers: Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to
|
There was a problem hiding this comment.
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 two refactor concerns and ruled them out: break_line dropping the explicit last_was_newline = true is covered because write_raw(b"\n") sets it from the trailing byte (src/md/ansi_renderer.rs:1096), and the Li/P/Hr arms that call write_indent() before their first write_styled do not double-indent, since write_indent now clears last_was_newline when it draws and returns early (leaving it unchanged) when the indent is zero, so begin_line is a no-op either way. The #[cold]/#[inline(never)] cost on the first write of each line was also looked at and judged a one-test-per-write change, not a per-byte one.
Extended reasoning...
The change reworks indent emission in the Markdown ANSI renderer (src/md/ansi_renderer.rs) by deferring quote bars and list indent to the first inline write of a line, touching write_wrapped, write_styled, write_no_wrap, enter_span, the raw-HTML arm, write_indent and the two Kitty image emitters, with new tests in test/js/bun/md/md-edge-cases.test.ts and test/cli/run/markdown-entrypoint.test.ts. It touches no security-sensitive surface. Three inline findings were posted (indent-only lines from empty links, two hand-inlined copies of begin_line, and a colored trailing-reset case the new test snapshots), so the inline comments already signal that a human look is needed; this note only records the state-tracking and double-indent concerns that were checked and ruled out.
…ine() where it is spelled out The two tests for an image that Kitty draws compare the text without the escapes where the image ends the paragraph. The reset that ends a paragraph comes behind the line break of the image, on main too, and the tests no longer hold that order. write_wrapped and write_styled spell the test of begin_line() out with their emptiness test. Their comments now say so. The call behind a test costs more instructions per render.
Problem
Bun.markdown.ansi()draws an HTML block in a list item at column 0 and moves the next marker right:- a\n <div>\n- b\nprints* a\n<div>\n * b\n(type-fest readme).write_wrapped(src/md/ansi_renderer.rs:764,:789) writes the next line's indent (quote bars, list spaces) after each line break, also when no line follows.Fix
begin_line()).write_indent()clearslast_was_newline, so a line with only an indent ends like any line.test/js/bun/md/md-edge-cases.test.ts(81 fail on main), one CLI test, all oftest/js/bun/md/.Background
"\n", also the last.Downsides
.mdfiles: 6 for the HTML block (none in a block quote), 5 for text behind a code block.columns > 0a render takes 0.3% (README.md) to 2.4% (2,000 HTML blocks) more instructions..text: +512 bytes.[]()alone behind a code block in a tight list item prints a line of only the indent. 0 of 4,598 files.Notes
Where this comes from. No user reported it and there is no issue. A review comment on #45611 asked about the layout of an indented HTML-block line in
ansi(). That layout is not changed here:columnsis documented for paragraphs, headings and the rule, and no text is lost. A read of the same path showed that an HTML block in a list item or a block quote is drawn outside of it, at every width, on 1.4.3 and on main. #46119 names the tight-item part as a separate renderer change. #46119 lists oneansi()case that it makes worse (cat column 0 behind a block in a tight item). On this branch the same order of events (- a\n > - # b\n c\n) putscat the indent of the item.What changes (
Bun.markdown.ansi(input, { colors: false }), the same atcolumns: 0and80):- a\n <div>\n- b\n* a\n<div>\n * b\n* a\n <div>\n * b\n> <div>\n<div>\n|| <div>\n1. <link to issue>\n2. <link to issue>\n\nafter\n1. \n<link to issue>\n 2. \n<link to issue>\n \nafter\n1. \n <link to issue>\n 2. \n <link to issue>\n\nafter\n- > <div>\n > x\n > </div>\n- next\n* \n\n<div>\n| x\n| </div>\n| * next\n* \n\n| <div>\n| x\n| </div>\n * next\na, a fenced code block, thenbbat column 0bat column 4- a \n- next\n* a\n * next\n* a\n * next\n> []()\n|| \nEvery kind of output change.
at the end of a paragraph, a hard break that ends link text or image alt text, a space at the wrap column, an image that Kitty draws at the end of a paragraph.[](), or text of control characters only). What follows is then where it is without it. On main the next list marker goes behind the left-over indent:- a\\\n []()\n- next\nprints* a\n * next\non main and* a\n \n * next\nhere.Not changed.
columns > 0.ansi()expectations that markdown: write code and HTML lines as source slices, so a leading tab stays a tab #45611 adds hold on this branch (9 of 9).bun lint --explaincalls the same function. It has no test here.Left as it is (the same bytes on main and here):
- x `a\n b` y\n- next\nprints* x a\nb y\n * next\n. 234 such spans are in 66 of the 4,598 files. 26 files have one in a list item,CLAUDE.mdof this repository among them (lines 13 and 14). The cause is another one:text()writes the code text whole, so its line break does not passbreak_line. Open Bun.markdown: implement the latexMath and underline options #39493 changes that arm. The change needs its own numbers, because it moves where lines wrap.> # a b\nprints| a\nb\n| ===\n, and a cell with breaks its row. The heading buffer and a cell are written whole. A setext heading with a hard break is the same case.1. step\n\n > **Note**\n > careful\n\n2. next\nprints1. step\n\n| Note careful\n 2. next\n.write_indent()writes all bars, then all spaces. 8 of the 4,598 files have a block quote in a list item.a \nprintsa\n\x1b[0m, and<div>\nprints\x1b[2m<div>\x1b[22m\x1b[2m\n\x1b[22m. Such output does not end in\n. Nothing of it is visible. To change the order, the line break has to wait for what follows it, and the bytes outside of a container change.How it works.
last_was_newlineis true while nothing is on the current line.write_indent()now clears it when it writes an indent. Before, a line with only an indent counted as empty, soensure_newline()did not end it and the next block was drawn behind it.begin_line()is called by the writers that can put the first byte on a line:write_wrapped,write_styledwith text,write_no_wrap,enter_span(before the span opens, so that its style and link do not cover the indent) and the HTML arm oftext(before the dim style).break_line(rest)writes the indent at once whenrestof the same text follows. That keeps the test off the path of a word.Li,Hr,P, the flush functions,wrap_break) are not changed. The two Kitty writers lose theirwrite_indent().write_wrappedandwrite_styledspell the test ofbegin_line()out, with their emptiness test in the same condition. A call of the helper behind the emptiness test costs more: README.md at 80 columns +0.48% against +0.29%, 40,000 spans +1.58% against +0.74% (the same main binary).in_main_flow()is the condition thatwrite_styledandwrite_no_wraphad inline.write_wrappedfinds the line break withstrings::index_of_char_usize, not byte by byte. Its loop body changed forbreak_line, and the byte loop got about 1 instruction per byte slower or faster with each version of that body.Tests.
md-edge-cases.test.ts: 81 fail on main and on 1.4.3. The other 25 end inas before. They pass on main, and each fails when one part of the fix is taken out.markdown-entrypoint.test.tsfails on main. The snapshot file does not change.Measurements. Main is 9634520. Release builds of both. 4,598 distinct
.mdfiles: the tracked ones, and those oftest/node_modules,node_modulesandvendor. 1,801 of them are compiler fixtures (.expect.md) with no list and no block quote. 1,818 have a list, 531 have a block quote..mdfiles (colors off and on,columns0 and 80, and colors with hyperlinks): 6 with an HTML block in a list item (3 prompt files of this repository, the type-fest readme at 2 versions, undici'sClient.md), 5 with text behind a code block in a tight list item (uglify-js, xml2js, fastifyServer.mdandTypeScript.md, google/benchmarkreducing_variance.md), 0 other, 0 unclassified. Colors-on only: 0. No file has an HTML block in a block quote.perf_event_opencounters in user mode, medians of 10 interleaved runs. The highest spread of the 10 runs is 0.08% of the median.valgrindis not installed on the build machine.columns: 0**spans**The decreases at
columns: 0come from the line-break search, which takes the place of a byte loop, not from the fix. The HTML-block rows are the highest: each line of an HTML block outside of a container tests for an indent twice and finds none to write.write_indent()calls per render of README.md at 80 columns: main 541, this branch 570. The 29 more return at once, because there is no indent to write outside of a container. The test for an empty line runs 3,937 times per render: once per call ofwrite_wrapped(410),write_styled(3,112),enter_span(359) and per HTML text (56). 52 of them find one. Per word: 0. (gdbhit counts on debug builds.)ansi()render of README.md: 349mallocand 23realloccalls on main, 349 and 23 on this branch (AddressSanitizer exit statistics of the debug builds, 101 renders minus 1, divided by 100).size_of::<AnsiRenderer>(): 376 bytes on both. The change adds no field..text: 81,932,581 bytes (main 81,932,069).bun_md::ansi_renderersymbols: 44,800 bytes (main 44,475). The stripped binary has the same size, 107,525,856 bytes.src/md/ansi_renderer.rs: +62/-40 in 17 hunks. 0 of the 12write_indent()calls that start a block's line are touched.bun file.md: do not fetch remote images in headings and table cells #46094. Conflict regions bygit merge-file: 2 withbun file.md: do not fetch remote images in headings and table cells #46094 (both wherein_main_flow()takes the place of the repeated condition), 1 with Bun.markdown.ansi: show image URL and cap Kitty image width #29131 (it has that one against main), 0 with the other eight. Afterbun file.md: do not fetch remote images in headings and table cells #46094,in_main_flow()must readself.leaf, and its comment that the Kitty writers write the indent is no longer true. Whichever lands second owns both.Self-review. 13 concerns, 12 addressed. The largest: the first version added a field,
indent_pending, that mirroredlast_was_newline. It is gone. That also removed a case that was worse than main (a paragraph of an outer list item drawn in a nested item behind a link that draws nothing). Not done: a PR of its own for the line break in a code span. It is another cause and another behaviour change, and it is named above with the arm that #39493 changes.