fix(make-pdf): give TOC headings stable ids so --toc anchors and page numbers resolve#1690
Open
officialasishkumar wants to merge 1 commit into
Open
Conversation
buildTocBlock minted a toc-N id per heading purely for its own markup (the <a href="#toc-N"> link and the data-toc-target="toc-N" page span), but no heading in the rendered body ever received that id. Headings come straight from marked (which emits <h1>One</h1> with no id) and wrapChaptersByH1, so every TOC anchor was a dead in-page link and every page-number target was unresolvable, leaving Paged.js nothing to count pages against. Replace extractHeadings with assignTocIds, which walks the H1-H3 headings in document order, assigns id="toc-N" to each (reusing an existing id when the heading already declares one, so no duplicate id is emitted), and returns both the id-annotated body HTML and the heading list. render() uses the annotated HTML for the body when a TOC is requested and builds the TOC from the same list, so anchors and page-number targets resolve to real headings. With no TOC the body is untouched (no ids injected), so non-TOC output and the combined-features gate stay byte-identical. Add regression tests asserting the anchors and data-toc-target ids resolve to heading ids in document order, that an existing heading id is reused without duplication, and that no ids are injected when the TOC is off. Fixes garrytan#1689
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
make-pdf --tocbuilt a table of contents whose entries linked to, and whose page-number spans targeted, element ids (toc-0,toc-1, ...) that were never assigned to any heading. Every TOC anchor was a dead in-page link and every page-number target was unresolvable, so Paged.js had nothing to fill the.toc-pagespans against.Fixes #1689.
Current behavior on upstream main
On
origin/main(920a13a1):The TOC entries point at
#toc-0/#toc-1/#toc-2, but no heading in the body carries any of those ids (they come straight frommarkedandwrapChaptersByH1, neither of which assigns ids).Fix
Replace
extractHeadingswithassignTocIds, which walks the H1-H3 headings in document order, assignsid="toc-N"to each, and returns both the id annotated body HTML and the heading list.render()uses the annotated HTML for the body when a TOC is requested and builds the TOC from the same list, so anchors anddata-toc-targetids resolve to real headings. A heading that already declares an id keeps it (the TOC points at that id), so no duplicate id is emitted.With no TOC the body is untouched (no ids injected), so non-TOC output is byte-identical to before.
Validation
After the fix:
Tests (the make-pdf-gate unit step,
bun test make-pdf/test/*.test.ts):Three new regression tests cover: anchors and page targets resolving to heading ids in document order, reuse of an existing heading id without duplication, and no id injection when the TOC is off.
The P0 combined-features gate (
make-pdf/test/e2e/combined-gate.test.ts) runsgenerate ... --quietwith no--toc. Since the change is entirely behindif (opts.toc)and--tocdefaults to false, the gate's rendered HTML (and extracted text) is unchanged.Scope
make-pdf/src/render.tsonly (render,buildTocBlock, andextractHeadingstoassignTocIds), plus three regression tests. No template, VERSION, or CHANGELOG changes.