Repository navigation
Leave a long s unfolded in OrdinalIgnoreCase, as StringComparison does [patch] - #80
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…s [patch]
char.ToUpperInvariant('ſ') is 'S', but StringComparison.OrdinalIgnoreCase
leaves U+017F alone, so NaturalStringComparer.OrdinalIgnoreCase sorted
"ſtraße2.txt" before "Tag1.txt" where OrdinalIgnoreCase puts it after. Fold
through one helper that keeps U+017F as it is, in both Fold and CompareFolded.
The test project now runs with invariant globalization off: ktsu.Sdk turns it
on, and invariant casing already leaves U+017F alone, which hid the bug from
the tests.
Fixes #79
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017vfJW66FJ9HR9vCd1x5h4H
|
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.
Fixes #79
What was wrong
NaturalStringComparer.OrdinalIgnoreCasesays it folds text "asStringComparison.OrdinalIgnoreCasefolds it", butFoldandCompareFoldedboth callchar.ToUpperInvariant. Under ICU,char.ToUpperInvariant('ſ')(U+017F) returns'S', whileOrdinalIgnoreCaseleaves U+017F alone. SoCompare("ſ", "t")came out negative, and["Tag1.txt", "ſtraße2.txt", "Rose1.txt"]sorted differently fromStringComparer.OrdinalIgnoreCase.Change
FoldCase(char)helper. It ischar.ToUpperInvariantexcept for U+017F, which it leaves unchanged. BothFoldandCompareFoldednow call it, so the two fold paths can't drift apart.Sorting.Test.csprojsets<InvariantGlobalization>false</InvariantGlobalization>. ktsu.Sdk turns invariant globalization on, and invariant casing already leaves U+017F alone. Under it the bug can't reproduce, and the new tests passed with or without the fix. Apps run under ICU, so the tests now do too.Tests
Compare_IgnoringCase_LeavesLongSUnfoldedAsOrdinalIgnoreCaseDoeschecks thatCompare("ſ", "t") > 0. For"s","S","t"and"T", both argument orders must agree in sign withstring.Compare(..., OrdinalIgnoreCase). It also checks that the issue's three file names sort the wayStringComparer.OrdinalIgnoreCasesorts them.Compare_IgnoringCase_FoldsEveryBmpLetterAsOrdinalIgnoreCaseDoescompares every BMP letter against ten probes and requires the sign to matchOrdinalIgnoreCasewherever that result is non-zero. This confirms that U+017F is the only letter where the two folds disagree.Note on #78
Open PR #78 (code-point fold for letters outside the BMP) touches the same fold paths, so whichever PR merges second will need a small rebase. If #78 lands first, its code-point fold should also go through
FoldCasefor BMP characters.Generated by Claude Code