Conversation
Add a Substring value to SearchType that matches the query anywhere within a string, case- and diacritic-insensitive, backed by a raw-string index scanned with CompareInfo.IndexOf. The existing Exact/Prefix/FullText modes are unchanged.
| return Enumerable.Empty<T>(); | ||
| CompareInfo ci = CultureInfo.InvariantCulture.CompareInfo; | ||
| return raw.Where(kv => ci.IndexOf(kv.Value, text, | ||
| CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace) >= 0).Select(kv => kv.Key); |
There was a problem hiding this comment.
we might need to consider if this will work correctly for us, or if we need to create a manged wrapper in icu-dotnet for string search so we can use what ICU provides for this use case.
There was a problem hiding this comment.
It seems that this is the same behavior as FWLite's search, so hopefully it will be a good way to go?
I did find another behavior to include, see the "Fold diacritics only when the search term has none" change here.
hahn-kev
left a comment
hahn-kev
left a comment
There was a problem hiding this comment.
This looks good. I left some suggestions, nothing blocking.
One thing, since the issue specifically calls out matching how FW Lite search works, here's some of our tests which would be relevant here
https://github.com/sillsdev/languageforge-lexbox/blob/bbcc058f267a789302b9570e9542d30968199a3f/backend/FwLite/MiniLcm.Tests/QueryEntryTestsBase.cs#L412-L455
Up to you how closely you try to match what FW Lite does.
|
Thank you! I'm looking into getting some of the enhancements you suggested in. |
- Build the sort-key index only in the search types that use it, so Substring no longer creates and discards an unused index (in both Add and Search). - Skip indexing null or empty text in Add instead of coercing it to an empty string. - Replace KeyValuePair<T, string> in the raw index with a named SubstringEntry struct for readability. Co-Authored-By: Claude Opus <noreply@anthropic.com>
- Fold diacritics for substring only when the query itself has none, so an unmarked query matches accented text but an accented query is specific. Added tests inspired by FWLite mirroring its SuccessfulMatches/NegativeMatches and NFC/NFD cases. Co-Authored-By: Claude Opus <noreply@anthropic.com>
thejambi
left a comment
thejambi
left a comment
There was a problem hiding this comment.
@thejambi made 3 comments.
Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on hahn-kev).
| return Enumerable.Empty<T>(); | ||
| CompareInfo ci = CultureInfo.InvariantCulture.CompareInfo; | ||
| return raw.Where(kv => ci.IndexOf(kv.Value, text, | ||
| CompareOptions.IgnoreCase | CompareOptions.IgnoreNonSpace) >= 0).Select(kv => kv.Key); |
There was a problem hiding this comment.
It seems that this is the same behavior as FWLite's search, so hopefully it will be a good way to go?
I did find another behavior to include, see the "Fold diacritics only when the search term has none" change here.
| } | ||
|
|
||
| public T Item { get { return m_item; } } | ||
| public string Text { get { return m_text; } } |
There was a problem hiding this comment.
you could simplify this to public string Text => m_text;
hahn-kev
left a comment
hahn-kev
left a comment
There was a problem hiding this comment.
Nice! Looks great, I just left one minor suggestion, feel free to ignore it.
thejambi
left a comment
thejambi
left a comment
There was a problem hiding this comment.
@thejambi reviewed 2 files and all commit messages.
Reviewable status: 0 of 2 files reviewed, 5 unresolved discussions (waiting on hahn-kev).
The substring search compared strings with .NET's InvariantCulture CompareInfo, which does not treat a composed character and its decomposed form as canonically equal on every runtime. Mono (net462 on Linux) missed such matches, failing SubstringMatch_isNormalizationInsensitive in CI while Windows and .NET 8 passed. Normalize both the stored text and the query to NFD so we always compare same-form strings. The diacritic-folding behavior is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Add a Substring value to SearchType that matches the query anywhere within a string, case- and diacritic-insensitive, backed by a raw-string index scanned with CompareInfo.IndexOf. The existing Exact/Prefix/FullText modes are unchanged. This change enables a change in FieldWorks to use this new SearchType, see sillsdev/FieldWorks#1069
https://jira.sil.org/browse/LT-22524
This change is