fix: fold Latin diacritics so accented comments match unaccented keywords - #58
Open
dchaudhari7177 wants to merge 1 commit into
Open
fix: fold Latin diacritics so accented comments match unaccented keywords#58dchaudhari7177 wants to merge 1 commit into
dchaudhari7177 wants to merge 1 commit into
Conversation
…ords A comment reading 'qual o PREÇO?' did not match a keyword of 'preco', so a creator had to guess every accent variation their audience might type. Fold in normalizeCommentText, which already runs over both the keyword and the comment, so one change covers both sides. Scoped to Latin: a blanket strip of every combining mark turns Cyrillic й into и and Japanese が into か -- different letters, not accented ones -- and in Devanagari, Thai and Arabic the marks are vowels rather than decoration. NFD puts the base character immediately before its mark, so the scope is one lookbehind on [A-Za-z]. Recomposed with NFC afterwards, so untouched scripts come out byte-identical.
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.
Closes #20
Reproduced first on
master:The change
foldLatinDiacriticsapplied at the top ofnormalizeCommentText. As you noted, that function already normalises both the keyword and the comment, so one change covers both sides.The scope you asked for falls out of
NFDdirectly: decomposition places the base character immediately before its combining mark, so "only drop a mark whose base is Basic Latin" is a single lookbehind. No script table to keep in sync.I added the trailing
NFCso the function's output stays canonical rather than half-decomposed. Matching would work without it — both sides run through the same normaliser — but it means every untouched script comes out byte-identical to its input, which is a much easier property to assert than "equivalent under some normalisation form".Acceptance criteria
precomatchesPREÇO,preço,PRECOcafematchescafé;ubermatchesüberйis not folded toиnormalizeCommentText('й') !== normalizeCommentText('и')がis not folded toかTwo extra cases worth having:
preçoalso matches an unaccentedprecocomment, since the fold is symmetric; andİstanbul(U+0130, a precomposed capital with dot above) folds correctly, becauseNFDdecomposes it before the lowercase step.Tests
A
Latin diacritic foldingblock inscripts/test-unit.ts: the eight matching cases as a table, a dedicated test each for the Cyrillic breve and the Japanese dakuten asserting both non-equality and non-matching, a loop asserting six non-Latin strings survive normalisation byte-for-byte, and one guarding that folding widens what counts as equal without widening what counts as a boundary —cafestill does not matchcafeteria.Reverting only
src/match.tswhile keeping the tests fails 7 of the 42.Verification
npm test— 42 pass, 0 fail.npm run typecheck— clean. No new dependency.Note this branches from
masterindependently of #56 (issue #19); both touchsrc/match.tsbut in different functions, so whichever lands first, the second is a clean rebase.