-
Notifications
You must be signed in to change notification settings - Fork 0
Raise or parse correctly where the docs Markdown parser silently mangled input #534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
dormouse-bot
wants to merge
3
commits into
main
Choose a base branch
from
fix/docs-parser-silent-mangling
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
fe1e6b1
fix(website): raise or parse correctly where the docs parser silently…
dormouse-bot cea9b88
fix(website): close the remaining silent-mangling gaps the review found
dormouse-bot eedff36
fix(website): require a pipe in the table delimiter row so `---` stay…
dormouse-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isDelimiterRow's regex makes every|optional, so a bare---or:---satisfies this lookahead.Intro/a | b/---therefore breaks the paragraph here and gets consumed by the table branch as a two-column header with a one-elementalignand no rows — wherefe1e6b1raised the setext error and GitHub renders an<h2>. Fixing it at the arm would leave the block-level table branch's identical hole open, so the fix belongs inisDelimiterRow(return row.includes('|') && /^\s*\|?…/.test(row)), per the review body.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in
eedff36— applied atisDelimiterRowas suggested, so the block-level table branch's identical hole (the barea | b/---case, which mangled the same way onmain) closes with it.Both shapes now raise
setext heading underline — use an ATX \#` heading, at line 3 and line 2 respectively. Confirmed the regression first by running the three parser revisions against the same input:0ba3d54folded it into one paragraph,fe1e6b1raised,cea9b88produced the two-column/one-align` table you describe.website/is green (18 files, 166 tests — the one added case pins both shapes), as arelint:specs, its self-test, andlint:public-docs. Regeneratingdocs.{guide,selfhost,cli,skill}.jsonwith this parser and withmain's gives byte-identical files, so the PR body's claim survives.No spec change: the governing rule is already in
docs/specs/website-docs.md-> "Markdown parsing" as Headings are ATX only, and the reason the pipe is required constrains this one module, so it lives as a docstring atisDelimiterRowalongside theTHEMATIC_BREAKordering note.The
align-length case one step further out — a delimiter row that has a pipe but whose cell count disagrees with the header (a | bover| ---), which GFM also says is not a table — is untouched and still mangles. It carries no setext ambiguity, so it is not the shape this PR set out to close; noting it rather than widening the diff.