Do not fold dotless i to ASCII i in reference labels - #308
Merged
Merged
Conversation
DePasqualeOrg
marked this pull request as ready for review
September 7, 2026 16:07
6 tasks
|
@jgm there was some conversation about this in micromark/micromark#232. Personally I have no strong opinions on where this should go. Spec/cmark. Or all the JS projects. |
Member
|
looks good to me, thanks. |
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.
This PR was developed with Claude Code, but solves a real issue I encountered with this library and is based on verified evidence. It underwent multiple rounds of automated review. The following description was produced by Claude Code. I’m happy to make any changes as needed.
The spec matches link labels under Unicode case fold.
normalizeReferenceapproximates that with.toLowerCase().toUpperCase(), which merges dotless i (U+0131) with ASCIIIandi, because'ı'.toUpperCase()is'I'.CaseFolding.txthas no entry for U+0131 at all, so it folds to itself. The only nearby mapping goes the other way, and0049; T; 0131is the Turkic profile, which CommonMark does not specify.This is a regression rather than a deliberate behavior. The generated table removed in b8fc0c7 for #168 left dotless i alone, and the changelog entry describes the replacement as an accurate Unicode case fold, which it is for every character but this one.
cmark keeps the two labels distinct:
In commonmark.js the second definition collides with the first, and the earlier one wins.
Dotless i is the only character affected. Comparing normalization output across every BMP codepoint before and after this change, U+0131 is the one that differs, so labels without it keep exactly the old expression.
Three regression cases are added: a bare
[ı]against an[I]definition, which no longer resolves; the two-definition collision above; and[FIQ]/[Fıq], which pairs dotless i with characters that still need folding. All three fail againstmaster.The
indexOfguard is purely an optimization; thereplacealone is correct for every input. Performance is unchanged, and since #168 weighed it, it was measured here too. Amortized over 2,000 parses per sample,bench/samples/README.md(the defaultmake benchinput) moves -0.7%, and the two reference-heavy samples land within a few percent either way across repeated sessions with no consistent direction.This does change rendering for existing documents: a
[ı]reference written against an[I]definition now finds no definition and renders as literal text. That is the intent, but it is a visible difference rather than an internal one.parser.refmapkeys change for these labels as well, though normalized labels reach neither the AST nor any renderer.