Skip to content

Match NFD text with NFC glob and regex filters, and the reverse [patch] - #199

Open
matt-edmondson wants to merge 4 commits into
mainfrom
fix/197-normalize-nfc
Open

matt-edmondson wants to merge 4 commits into
mainfrom
fix/197-normalize-nfc

Conversation

@matt-edmondson

Copy link
Copy Markdown
Contributor

Fixes #197

What changed

Fuzzy filtering compares in Unicode normal form C because it goes through ktsu.FuzzySearch, but glob and regex filtering compared raw strings. A name stored decomposed (NFD), the way macOS writes file names, disappeared in Glob and Regex mode and reappeared in Fuzzy mode.

  • A new ToNormalizationFormC helper returns a string that is already NFC unchanged, so ASCII and NFC text cost no extra allocation. A string that can't be normalized, such as one with a lone surrogate, is matched as it is instead of throwing.
  • Glob: the filter token is normalized in ResolveGlob before the cache key is taken, so the NFC and NFD spellings of one filter share a cache entry. The text token is normalized in IsGlobMatch. Normalization happens before ExpandGlobClasses, so Caf[éè]* works with either spelling inside the class. It also covers the public AnyTokenMatchesGlobFilter / AllTokensMatchGlobFilter entry points.
  • Regex: the text and the pattern are normalized at the top of DoesMatchRegex.

Tests

  • NFC filter against NFD text, and the reverse, for glob with *, ? and [...], under both case sensitivities and the default match option.
  • The same for regex under ByWordAny and ByWholeString, case-sensitive and case-insensitive.
  • Filter over mixed NFC/NFD items returns the same items for Glob, Regex and Fuzzy.
  • NFC and ASCII input come back as the same instance, and a lone surrogate is still matched.

With normalization stubbed out, 13 of the new cases fail. With the fix, every new test passes. dotnet test gives 282/283. The one failure, ReadmeTests.EveryMethodInTheReadmeApiReferenceExistsWithItsDocumentedDefaults, also fails on main (5f420d9) without this change. Branch fix/183-readme-api-test-compat-overloads appears to cover it.


Generated by Claude Code

Glob and regex filters compared raw strings, while fuzzy compares in
Unicode normal form C, so a macOS-style decomposed name vanished in Glob
and Regex mode but showed up in Fuzzy. Both sides are now brought to NFC
before glob and regex matching; NFC text is returned as is, and the glob
filter is normalized before its cache key is taken.

Fixes #197

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW3WKJyQMZaWCcyzKVK
…patch]

The README API test required every overload's same-named parameter to carry
the documented default. The binary-compatibility overloads from #166 have no
defaults by design, so main has been red since #178. They are marked
[EditorBrowsable(Never)] and are not what the README documents, so the default
check now skips them. A wrong default on a visible overload still fails.

Fixes #183

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW6FzBtSrY5LKUuDYce
(cherry picked from commit 9328e00)

Copy link
Copy Markdown
Contributor Author

CI on 0143ded failed ci / .NET / Test on all three OSes. The failing test was ReadmeTests.EveryMethodInTheReadmeApiReferenceExistsWithItsDocumentedDefaults. It isn't caused by this PR: the scheduled CI run on main (5f420d9) fails the same way, and the test passes or fails identically with or without this change.

A fix already exists on open branch fix/183-readme-api-test-compat-overloads (9328e00), so I've cherry-picked it here as 0aa598d rather than leave this PR red until that one merges. With it, dotnet test passes 283/283 locally.


Generated by Claude Code

Copy link
Copy Markdown
Contributor Author

The SonarCloud quality gate (Maintainability C on new code) isn't caused by this PR. SonarCloud last analysed main on 2026-11-04, at 837b460. Every scheduled CI run on main since then has failed on the README test, so that analysis never refreshed. This PR is therefore compared against a five-day-old baseline. The issues counted as "new code" include the four S3427 BLOCKERs from the TextFilter.BinaryCompatibility.cs overloads (#166) and the S3776 in ExpandGlobClasses, neither of which this diff touches. main's own analysis already lists them. The only findings on lines this PR adds are three INFO-level MSTEST0068 suggestions in the new tests. Fixing those would not get the gate to A.

What clears it is a green main that SonarCloud can re-baseline against. That is #186, which this PR already carries as a cherry-pick. Once #186 merges, re-running this PR's CI should give a fresh comparison. #200 is in the same state.


Generated by Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW5jpiuVuBRXSgitSY1
# Conflicts:
#	TextFilter.Test/TextFilterTests.cs
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Glob and Regex filters miss decomposed (NFD) text that Fuzzy finds: "Café*" doesn't match a macOS-style "Café menu.pdf"

2 participants