feat: make generated heading ids optional - #46
Merged
Merged
Conversation
Benchmark ResultsComparison of Open to see the benchmark resultsNo benchmark changes above ±5%. Generated by phpbench against commit 66906a1 |
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.
Part of #13 — your first example there is
$markdown->getRule(HeadingRule::class)->showAnchors(true);, and this is the rule half of it.Every heading gets an id slugged from its content, with no way to opt out.
HeadingRulenow takesgenerateIds:An id written explicitly is kept either way, so
## A heading ## custom-idstill renders<h2 id="custom-id">. Default istrue, nothing changes for anyone who doesn't ask.A public promoted property rather than a
showAnchors()setter: it reads the same at construction (new HeadingRule(generateIds: false)) and after the fact ($markdown->getRule(HeadingRule::class)->generateIds = false), so it works whichever way #13 lands. Happy to rename it to yourshowAnchorsif you prefer that vocabulary — I pickedgenerateIdsonly because the rule emits anidattribute rather than an anchor element.One gap, and it is #18's
Configuring the rule reaches the document, but not a heading nested inside another token, because
DivTokenstill hard-codesnew HeadingRule():#45 closes that, by resolving a token's default rules through the configured set. This PR doesn't depend on it and is mergeable on its own; the two together make the option hold everywhere.
Why I care
I maintain an integration that adds heading ids later in its own render chain, from the whole page rather than from one block, so the parser has to leave them out. Today that means stripping the attribute back out of the HTML with a regex.
Measured on a personal dataset of real-world Markdown: automatic ids are the single biggest reason a document does not already render identically through plain Tempest — ahead of every correctness bug reported in #37 combined. This one option moves the share of documents that match from roughly three in five to roughly three in four.
composer test(331 tests),lintandanalyseare clean.