ndg-commonmark: align TOC anchors with heading ids for escaped markup#218
Conversation
NotAShelf
left a comment
NotAShelf
left a comment
There was a problem hiding this comment.
Thank you for the PR, and thank you for the agent disclosure! I've left one quick note, which is not a blocker, and completely optional to address.
If you could just add a CHANGELOG entry under 2.9.0 # Fixes section, we'll be golden.
KiaraGrouwstra
force-pushed
the
fix-heading-id-decode-entities
branch
2 times, most recently
from
93be656 to
be3e451
Compare
NotAShelf
changed the title
|
There seems to be a failing test, could you fix it before I can merge? |
Headings whose text contains markup characters (e.g. an option named `environments.<name>.deployment`) had a table-of-contents anchor that did not match the heading's `id`, so "jump to header" landed nowhere. The auto-generated heading `id` slugifies the *rendered* HTML, in which comrak has escaped the brackets to `environments.<name>.deployment`, giving `environments--lt-name-gt--deployment`. The table-of-contents slugified the raw inline text instead, giving a different slug. Close the gap on the table-of-contents side via a new `slugify_heading` helper that escapes the text the same way comrak does before slugifying. The heading `id` is left unchanged so existing deep links keep resolving. Adds a regression test asserting the TOC anchor matches the rendered heading `id`. Assisted-by: Claude:claude-opus-4-8
KiaraGrouwstra
force-pushed
the
fix-heading-id-decode-entities
branch
from
be3e451 to
194f146
Compare
|
Thanks for the heads-up! I updated that test to match this change as well now. |
NotAShelf
left a comment
NotAShelf
left a comment
There was a problem hiding this comment.
In hindsight installation--amp--setup is probably not the most intuitive heading ID, but I don't think this is a reason to block this PR any further. I'll pinpoint the semantics once I have a little bit more free time. Thank you for your good work, LGTM
|
i agree! and the original reconciliation i found was in fact to just add a decode_html_entities call to the id generator. |
2 participants
Encodes HTML entities in heading ids in the TOC anchors to unbreak jump-to-header for headers with auto ids based on HTML entities in their content (e.g.
.<name>).This solution intentionally preserves the current (uglier) auto-ids (rather than resolving the gap the other way around) to prevent existing deep-links from breaking.
Closes #217.
Disclaimer: I used a coding agent in the creation of this patch.