Filter the Command Palette in English in addition to locale #19166
Conversation
| static bool shouldShowSubtitles = [] { | ||
| auto ctx = winrt::Windows::ApplicationModel::Resources::Core::ResourceContext::GetForViewIndependentUse(); | ||
| auto qv = ctx.QualifierValues(); | ||
| auto lang = qv.TryLookup(L"language"); | ||
| return lang != L"en-US"; | ||
| }(); |
There was a problem hiding this comment.
Yeah, that's not great. This is actually another great example for what I said in my talk last week: If your base functions have bad performance you end up writing boilerplate caching logic to work around it everywhere. This is exactly that.
In any case, should this use til::starts_with_insensitive_ascii("en-")? 🤔
| auto itemSubtitle = _Item.Subtitle(); | ||
| int32_t subtitleWeight = 0; | ||
| std::tie(subtitleSegments, subtitleWeight) = _matchedSegmentsAndWeight(_pattern, itemSubtitle); | ||
| weight += subtitleWeight; |
There was a problem hiding this comment.
OPEN DISCUSSION (cc @e82eric) how do we combine weights when we have multiple haystacks
There was a problem hiding this comment.
for the localization use case, looking at the screenshots, I would lean towards using the max of the 2 scores. I think where "split" gets matched in both the localized and English haystacks adding them would cause the items with English chars/matches in the localized version to score unfairly high compared to the ones that don't.
There was a problem hiding this comment.
Excellent excellent call. I've done this :)
703afff to
497b8f2
Compare
996b06c to
e867912
Compare
df38587 to
38b9a7d
Compare
| static bool shouldShowSubtitles = [] { | ||
| auto ctx = winrt::Windows::ApplicationModel::Resources::Core::ResourceContext::GetForViewIndependentUse(); | ||
| auto qv = ctx.QualifierValues(); | ||
| auto lang = qv.TryLookup(L"language"); | ||
| return lang != L"en-US"; | ||
| }(); |
There was a problem hiding this comment.
Yeah, that's not great. This is actually another great example for what I said in my talk last week: If your base functions have bad performance you end up writing boilerplate caching logic to work around it everywhere. This is exactly that.
In any case, should this use til::starts_with_insensitive_ascii("en-")? 🤔
| } | ||
|
|
||
| winrt::hstring CopyTextArgs::GenerateName() const | ||
| winrt::hstring CopyTextArgs::GenerateName(bool localized) const |
There was a problem hiding this comment.
Reading this makes me realize that a localization system should probably pass the context of the localization (i.e. the locale) as a parameter to make the translation function pure. I should probably do the same for Edit...
There was a problem hiding this comment.
If we want to do that correctly, we should need to use ResourceContext directly. I am willing to make that change, but it is much more complicated than the one we have.
|
One thing I keep wondering on this, is how useful the subtitles are for locales that only have a couple of difficult to type chars (non Latin script languages?). I think there might be some cases like the user is typing something in Spanish and gets a result from a match on the English subtitle that they didn't expect. It might be worth logging a future improvement to this to only show the subtitles for non Latin script languages (CJK, Cyrillic, Arabic, etc.) and then add the normalization logic into the fzf code to help with the non English Latin script languages. This is probably a rare edge case, but thought it would be worth asking. |
|
I think it's less about things which are hard to type, and more about having a common language -- it's easy to search a command palette using terms you find in documentation (for example) written in English and scattered about the web. |
|
ahh, got it, that makes sense. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
The bulk of this work is changing
Command::Name(and its descendants likeGenerateName) to support looking up names in English and in the local language.Refs #19130, #19131, #19132, #19165
Closes #7039