Skip to content

Require the table delimiter row's cell count to match the header - #539

Merged
nedtwigg merged 2 commits into
mainfrom
fix/docs-parser-table-cell-count
Sep 3, 2026
Merged

Require the table delimiter row's cell count to match the header#539
nedtwigg merged 2 commits into
mainfrom
fix/docs-parser-table-cell-count

Conversation

@dormouse-bot

Copy link
Copy Markdown
Collaborator

GFM does not recognise a table when the header and delimiter row have different cell counts, so a | b over | --- | is paragraph text. The paragraph-interrupting lookahead #534 added stops at isDelimiterRow, which validates the row's syntax in isolation, so that input parses as a table instead: the delimiter row disappears from the output entirely and the prose above it renders as a table header, with no error — the same silent-mangle shape #534 set out to close. startsTable now requires the counts to match, at both the lookahead and the block-level table branch, which had the identical hole from before #534.

Verified against main at 728b3c7: generate-docs.js produces byte-identical docs.{guide,selfhost,cli,skill}.json before and after, so nothing published changes.

The case, with the observed output
input:  Intro          main:   paragraph("Intro"), table(header=[a, b], align=[null], rows=[])
        a | b          this:   paragraph("Intro a | b | --- |")
        | --- |        GitHub: <p>Intro a | b | --- |</p>

main reaches this through the paragraph lookahead: interruptsParagraph ends the paragraph at a | b because a delimiter row follows, then the block-level branch builds a table from a header of two cells and a delimiter row of one. The bare a | b / | --- | form hits the block-level branch directly and mangles the same way on main and before #534.

MarkdownDocument.tsx never reads node.align, so the short align array does no rendering damage on its own; the visible damage is the dropped delimiter row and the restyled prose.

Why the count is taken without parseInline. startsTable runs speculatively on every paragraph line, and splitRow would raise UnsupportedMarkdownError on inline content the parser rejects — a paragraph line containing a pipe and, say, a setext-adjacent construct would throw during a lookahead that is only asking "does a table start here?". So splitRow's escape-aware loop is split into splitCells, which returns raw strings, and splitRow, which maps parseInline over them. The escaped-pipe handling the shortcut table needs stays in the shared loop.

Tests

Three cases in docs-parser.test.js. Two fail on main and pass here (reads a delimiter row whose cell count differs from the header as prose, keeps a mismatched delimiter row inside the blockquote it lazily continues); one (still parses a table whose header omits the outer pipes) passes both ways and guards the fix against over-correcting on the pipeless-border a|b / -|- form.

pnpm test in website/ is green (18 files, 169 tests), as are lint:specs and lint:public-docs.

No spec change: per the merge commit on #534, correctness within a construct the spec already claims is a parser concern, and the rule lives as the startsTable docstring.

Found by the review on #534, which landed two minutes before that PR merged.

…he header

GFM does not recognise a table when the header and delimiter row have
different cell counts, so `a | b` over `| --- |` is paragraph text. The
lookahead added in #534 stopped at `isDelimiterRow`, which validates the
row's syntax in isolation, so that input parsed as a table: the delimiter
row vanished from the output and the prose above it rendered as a table
header, with no error. The block-level table branch had the same hole
from before #534.

`splitRow`'s escape-aware loop is split into a raw cell-splitter that
`startsTable` can call speculatively on every paragraph line, where
`parseInline` would raise on inline content the parser rejects.
@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 3, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: fbcc5db
Status: ✅  Deploy successful!
Preview URL: https://797ea43e.mouseterm.pages.dev
Branch Preview URL: https://fix-docs-parser-table-cell-c.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

The fix is right and the direction is conservative: every mismatch now falls through to prose, which is what GitHub does. I re-derived the byte-identity claim independently — parsing all 85 Markdown files in the repo with the base parser and this one gives identical trees (77 identical, 8 raising the same error both ways), so nothing published moves. The align array is also now guaranteed to match the header length, since alignmentsFrom and splitCells split a delimiter row the same way and the delimiter regex admits no backslash.

One gap. The escape-aware count is the stated reason splitCells was extracted with the escape loop shared, but nothing pins it: replacing splitCells(line).length with a naive line.trim().replace(/^\|/, '').replace(/\|$/, '').split('|').length still passes all 169 tests and still leaves all 85 repo Markdown files byte-identical. Every escaped pipe in the repo sits in a body cell (docs/specs/shortcuts.md, vscode-ext/README.md), never a header, so the regression would land silently. Inline suggestion adds the case — it passes here and fails against that mutation.

Comment thread website/scripts/docs-parser.test.js
@nedtwigg
nedtwigg merged commit 6e05c47 into main Sep 3, 2026
10 checks passed
@nedtwigg
nedtwigg deleted the fix/docs-parser-table-cell-count branch September 3, 2026 21:55
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.

2 participants