fix: BL-15920 reopening with selected script#136
Conversation
| rawResults.map((r) => r.item), | ||
| code | ||
| )[0] | ||
| ? deepStripDemarcation( |
There was a problem hiding this comment.
I feel like our bracket demarcation (for bolding the substring that matches the query) is specific to our "defaultSearchResultModifier" and I don't like that it is getting baked in everywhere. It keeps tripping me up. If I were starting over I would try to see if we can do it at card render time instead. But here we are, it is already baked in in a lot of places
There was a problem hiding this comment.
And devin has just reported that there is real language data with square brackets...
So we should definitely look into revamping this...
components/language-chooser/common/find-language/searchForLanguage.ts:R114-119
deepStripDemarcation uses bracket characters that exist in real language data
The demarcation markers [ and ] (defined in matchingSubstringDemarcation.ts:5-6) are stripped from ALL string fields by deepStripDemarcation. Two entries in languageData.json contain legitimate brackets: Hebrew (heb) has a name ivrít ḥadašá[h] and Khalaj (kjf) has exonym Khalaj [Indo-Iranian]. When getLanguageBySubtag is called with a searchResultModifier for either of these languages, deepStripDemarcation at searchForLanguage.ts:114 would corrupt those fields (e.g., Khalaj Indo-Iranian instead of Khalaj [Indo-Iranian]). This is a pre-existing design issue with the marker choice rather than a bug introduced by this PR, but it's now surfaced in a new code path.
There was a problem hiding this comment.
There was a problem hiding this comment.
Can we do that separately? It would be pretty invasive; I recommend we put this current PR into 6.2 but don't put a redo of the demarcation system into 6.2
There was a problem hiding this comment.
Yes, definitely separate
andrew-polk
left a comment
andrew-polk
left a comment
There was a problem hiding this comment.
@andrew-polk reviewed 2 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on nabalone).
| rawResults.map((r) => r.item), | ||
| code | ||
| )[0] | ||
| ? deepStripDemarcation( |
There was a problem hiding this comment.
Yes, definitely separate
2 participants
This change is