Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore case when sorting command palette #8430

Closed
Don-Vito opened this issue Nov 28, 2020 · 4 comments · Fixed by #8432
Closed

Ignore case when sorting command palette #8430

Don-Vito opened this issue Nov 28, 2020 · 4 comments · Fixed by #8432
Assignees
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@Don-Vito
Copy link
Contributor

Description of the new feature/enhancement

I took me a while to understand why custom commands I add appear at the bottom.
The reason is that when sorting commands in command palette we do it not lexicographically and as a result consider a case.

IMHO the case should not matter in the palette (or at least case-insensitive sorting should be optionally available).

Proposed technical implementation details (optional)

Upon comparing the filtered commands, try to set both command names to lower case and then compare lexicographically.

@Don-Vito Don-Vito added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Nov 28, 2020
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Nov 28, 2020
@ghost ghost added the In-PR This issue has a related PR label Nov 28, 2020
@DHowett DHowett added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Nov 28, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Nov 28, 2020
@Don-Vito
Copy link
Contributor Author

Don-Vito commented Nov 29, 2020

@DHowett - actually I am not sure what to do 🤔

As @j4james mentioned correctly here, case is just a subset of a larger problem and will not solve accented chars, etc.

And the larger problem is described here: #6953 - we need to use correct locale for comparison.

When I last talked to @zadjii-msft we decided that probably we should not invest in it, since in any case we want to implement #7039 - that says not to localize the palette at all.

So now I am on the crossroads of:
1. Do nothing (hate this one)
2. Work on #7039 (though I am not sure how to advance there - are the US_English resources packed by default in addition to other languages?)
3. Work on #6953 - try to read the user locale and use it
4. Simply compare the strings in lowercase (will probably help only for ascii characters)

WDYT?

@j4james
Copy link
Collaborator

j4james commented Nov 29, 2020

Wow. I had no idea this issue was so complicated. But my understanding from reading #7039, is that VS is not just leaving the palette in English - it's listing both the native language as well as English, and allowing the search to match both of them. In that case, it still makes sense to sort with the user's native locale I would think.

If the plan is to not show the native language at all, then I guess a hardcoded US-EN locale may be best (assuming that is always available). But worst case I'd think the user's native locale would probably still be reasonable for sorting, even on English text - at least it can't be worse than ASCII ordering and seems neater than lowercasing everything (which can produce inconsistent results).

@zadjii-msft
Copy link
Member

For the record, this seems fine to me, we'll loop back around on the "always display the EN-US command name" thing later, let's just start with this since it's a good low-hanging fix

@ghost ghost closed this as completed in #8432 Nov 30, 2020
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Nov 30, 2020
ghost pushed a commit that referenced this issue Nov 30, 2020
@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8432, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants