Skip to content

Fix several issues in Command Palette#18486

Open
KrisVandermotten wants to merge 2 commits into
files-community:mainfrom
KrisVandermotten:CommandPaletteSuggestions
Open

Fix several issues in Command Palette#18486
KrisVandermotten wants to merge 2 commits into
files-community:mainfrom
KrisVandermotten:CommandPaletteSuggestions

Conversation

@KrisVandermotten
Copy link
Copy Markdown
Contributor

@KrisVandermotten KrisVandermotten commented May 14, 2026

Resolved / Related Issues

Closes #18395

  • The main problem was a bug in NavigationToolbarViewModel.UpdateCommandPaletteSuggestions, more specifically in the else branch loop. That else branch was completely removed. We now always replace all items in the OmnibarCommandPaletteModeSuggestionItems collection. This also results in simpler and more efficient code.
  • In fact, NavigationToolbarViewModel.UpdateCommandPaletteSuggestions has become so simple that it was inlined into its caller, PopulateOmnibarSuggestionsForCommandPaletteMode. Because of that change, we no longer need the intermediate newSuggestions collection, saving memory and resulting in a more responsive UI update.
  • The number of await Task.Yield() calls is reduced. There were too many, resulting is too much pointless overhead.
  • Building the commandsToProcess list has been optimized, especially for the case where the OmnibarCommandPaletteModeText search string is empty, i.e. the first list to be calculated when the command palette is opened.
  • Because the calculation of the the commandsToProcess list happens on a background thread, we now test whether OmnibarCommandPaletteModeText has not changed by the time the list calculation is finished, and prevent further processing of stale results if it did.
  • The NavigationBarSuggestionItem objects are now immutable. They were never mutated anyway. This also simplifies their construction.
  • The NavigationBarSuggestionItem.ActionIconSource property was removed as it was never assigned and always had the value null.
  • The NavigationBarSuggestionItem.PrimaryDisplay property was removed as it always had the same value as the Text property.
  • The NavigationBarSuggestionItem.SearchText property was removed as it was never read.
  • A NavigationBarSuggestionItem.Command property was added, storing the command used to construct the NavigationBarSuggestionItem. That property is used in NavigationToolbar.Omnibar_QuerySubmitted, eliminating the need to search for the command.
  • Because NavigationBarSuggestionItem is now immutable, it no longer inherits from ObservableObject and it no longer tries to raise pointless PropertyChanging and PropertyChanged events during its construction.
  • Also because NavigationBarSuggestionItem is now immutable, data binding in NavigationToolbar.xaml now uses Mode=OneTime instead of Mode=OneWay. As a result, the UI no longer needs to wire up listeners for events that will never be raised anyway.
  • Finally, in Omnibar.AutoSuggestBox_TextChanged, we set _textChangeReason to OmnibarTextChangeReason.UserInput when it's not OmnibarTextChangeReason.ProgrammaticChange and _userInput.Length == 0. This fixes the bug where clicking the X button at the right of the textbox to clear it did not cause a recalculation of the suggestions list.

As a result of the above

  • several bugs were fixed,
  • the code is significantly simplified,
  • and UI responsiveness and memory usage have improved considerably.

Steps used to test these changes

  1. Opened Files
  2. Opened the command palette using Ctrl-Shift-P and tested several scenarios:
    1. Searched for existing commands by typing terminal one character at a time, as well as removing characters using Backspace, and observed that suggestions are now correct.
    2. Executed commands after searching for them. Observed that the correct command is still executed when selecting the suggestion.
    3. Searched for non-existing commands by typing xyz. Observed that the NoCommandsFound message is still displayed correctly.
    4. Tried to execute non-existing commands such as xyz. Observed that the dialog box is still displayed correctly.
    5. Clicked the X button in the textbox to clear it. Observed that this now correctly causes suggestions to be recalculated.
    6. Type the full description of a command manually and execute it without selecting a suggestion. Observe that the correct command is still executed.

@KrisVandermotten
Copy link
Copy Markdown
Contributor Author

That failed test seems unrelated to my changes. Can you take a look @yair100 ?

@yair100
Copy link
Copy Markdown
Member

yair100 commented May 15, 2026

The test is passing now. That being said, this PR seems to include a lot of changes that weren't discussed beforehand. It will speed up the review process if this focuses on the fix needed for #18395. We can discuss the other changes as well, but they shouldn't be part of this PR.

@KrisVandermotten
Copy link
Copy Markdown
Contributor Author

That being said, this PR seems to include a lot of changes that weren't discussed beforehand.

True. I described them in the description of this PR.

It will speed up the review process if this focuses on the fix needed for #18395.

I made #18496. It fixes the one issue.

We can discuss the other changes as well, but they shouldn't be part of this PR.

I described them in the description of this PR. Feel free to discuss.

IMHO it's easier to discuss changes when you see them, instead of when they're still an abstract concept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Duplicate entries in command list

2 participants