Skip to content

fix: don't indent structuredText blocks that follow whitespace-only text - #329

Open
OsamaAnsar wants to merge 1 commit into
taoqf:mainfrom
OsamaAnsar:fix/structured-text-whitespace-between-blocks
Open

OsamaAnsar wants to merge 1 commit into
taoqf:mainfrom
OsamaAnsar:fix/structured-text-whitespace-between-blocks

Conversation

@OsamaAnsar

Copy link
Copy Markdown

structuredText puts a leading space on every block that follows a whitespace-only text node, which is what pretty-printed HTML produces.

parse('<div>\n  <h1>Title</h1>\n  <p>First para</p>\n  <p>Second <b>bold</b> para</p>\n  <ul>\n    <li>one</li>\n    <li>two</li>\n  </ul>\n</div>').structuredText
// before: " Title\n First para\n Second bold para\n one\n two"
// after:  "Title\nFirst para\nSecond bold para\none\ntwo"

parse('<p>a</p> <p>b</p>').structuredText   // before: "a\n b"   after: "a\nb"
parse('<p>a <br> b</p>').structuredText     // before: "a \n b"  after: "a\nb"

Cause: the whitespace-only branch in structuredText set prependWhitespace even when the current block was still empty, so the first text of the next block got a prefix. Block lines were also never trimmed, so a space before a <br> stayed as a trailing space.

Fix: only remember pending whitespace when the block already has content, and .trim() each line after the existing whitespace normalisation.

One behaviour change beyond the pretty-printed case: since lines are now trimmed, a single leading/trailing space inside a line is dropped for compact HTML too (<p> a</p> gave " a", now "a"; <p>a </p>b gave "a \nb", now "a\nb"). Output for inputs without such spaces is unchanged, and the existing structuredText tests pass untouched. I took that to be the same bug, but say so if you would rather keep it.

Tests: 4 added next to the existing #structuredText tests in test/tests/html.js (pretty-printed list/headings case, <p>a</p> <p>b</p>, <br>, and a guard that a single space between inline elements is kept). With only the src/ change reverted all 4 fail; with it they pass.

How I ran things: bun is not installed here, so I bundled src/index.ts with esbuild (css-select 5.2.2 and entities 8.0.0, as in bun.lock) and ran mocha on Node 22 against that bundle, skipping issues/280.js (it needs the published npm packages). main: 303 passing, 4 failing. This branch: 307 passing, same 4 failing. The 4 failures are artifacts of my setup, not of this change: the two Module Import tests and the two unclosed tag repair subprocess tests need ts-node / a built dist. tsc --noEmit is clean.

Not tested: bun run build / test:dist against a real dist, eslint, Node 24/26.

A whitespace-only text node set prependWhitespace even when the current
block was still empty, and block lines were never trimmed. With
pretty-printed HTML every block after a whitespace node therefore got a
leading space ('<p>a</p> <p>b</p>' gave 'a\n b'), and a space left before
a <br> stayed as a trailing space ('a \n b').

Only remember the pending whitespace when the block already has content,
and trim each line after normalising its whitespace.
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.

1 participant