From 3c6fa44bf2cd86d85a50125d83c7d1356e7f2b9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Pol=C3=A1=C5=A1ek?= Date: Thu, 10 Jul 2025 03:44:08 +0200 Subject: [PATCH] Prevent apps from appearing in top-level search when Installed apps extension is disabled (#40132) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary of the Pull Request Prevents installed applications from appearing in the top-level search when Installed Apps extension is disabled. Previously, application commands were still returned in the global search results even when the *Installed Apps* extension was turned off. To match user expectations, the search now respects the extension’s enabled state. - Added `IsActive` property to `CommandProviderWrapper` to indicate whether the provider is both valid and enabled by the user in the settings. - Updated `MainListPage` to verify that the provider for `AllApps` is active before including apps in filtered results. ## PR Checklist - [x] **Closes:** #39937 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized - [x] **Dev docs:** nothing to update - [x] **New binaries:** none - [x] **Documentation updated:** nothing to update ## Detailed Description of the Pull Request / Additional comments ## Validation Steps Performed Verified that the Installed app entries are shown in the top-level search only when the Installed apps extension is enabled. Verified that turning the Installed apps extension on or off has an immediate effect, and that the behavior persists after an application restart. --- .../CommandProviderWrapper.cs | 6 ++- .../Commands/MainListPage.cs | 47 +++++++++++++++++-- .../TopLevelCommandManager.cs | 43 ++++++++++++++--- .../AllAppsCommandProvider.cs | 4 +- 4 files changed, 89 insertions(+), 11 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs index f0044a33d6..2ca9c9eaab 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs @@ -40,6 +40,8 @@ public sealed class CommandProviderWrapper public CommandSettingsViewModel? Settings { get; private set; } + public bool IsActive { get; private set; } + public string ProviderId { get @@ -124,12 +126,14 @@ public sealed class CommandProviderWrapper { if (!isValid) { + IsActive = false; return; } var settings = serviceProvider.GetService()!; - if (!GetProviderSettings(settings).IsEnabled) + IsActive = GetProviderSettings(settings).IsEnabled; + if (!IsActive) { return; } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs index d60571643d..04277083ef 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs @@ -5,6 +5,7 @@ using System.Collections.Immutable; using System.Collections.Specialized; using CommunityToolkit.Mvvm.Messaging; +using ManagedCommon; using Microsoft.CmdPal.Ext.Apps; using Microsoft.CmdPal.UI.ViewModels.Messages; using Microsoft.CommandPalette.Extensions; @@ -25,6 +26,8 @@ public partial class MainListPage : DynamicListPage, private readonly TopLevelCommandManager _tlcManager; private IEnumerable? _filteredItems; + private bool _includeApps; + private bool _filteredItemsIncludesApps; public MainListPage(IServiceProvider serviceProvider) { @@ -64,7 +67,34 @@ public partial class MainListPage : DynamicListPage, } } - private void Commands_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) => RaiseItemsChanged(_tlcManager.TopLevelCommands.Count); + private void Commands_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e) + { + _includeApps = _tlcManager.IsProviderActive(AllAppsCommandProvider.WellKnownId); + if (_includeApps != _filteredItemsIncludesApps) + { + ReapplySearchInBackground(); + } + else + { + RaiseItemsChanged(_tlcManager.TopLevelCommands.Count); + } + } + + private void ReapplySearchInBackground() + { + _ = Task.Run(() => + { + try + { + var currentSearchText = SearchText; + UpdateSearchText(currentSearchText, currentSearchText); + } + catch (Exception e) + { + Logger.LogError("Failed to reload search", e); + } + }); + } public override IListItem[] GetItems() { @@ -119,12 +149,23 @@ public partial class MainListPage : DynamicListPage, _filteredItems = null; } + // If the internal state has changed, reset _filteredItems to reset the list. + if (_filteredItemsIncludesApps != _includeApps) + { + _filteredItems = null; + } + // If we don't have any previous filter results to work with, start // with a list of all our commands & apps. if (_filteredItems == null) { - IEnumerable apps = AllAppsCommandProvider.Page.GetItems(); - _filteredItems = commands.Concat(apps); + _filteredItems = commands; + _filteredItemsIncludesApps = _includeApps; + if (_includeApps) + { + IEnumerable apps = AllAppsCommandProvider.Page.GetItems(); + _filteredItems = _filteredItems.Concat(apps); + } } // Produce a list of everything that matches the current filter. diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs index 2bd6dbd365..eb04901868 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs @@ -26,6 +26,7 @@ public partial class TopLevelCommandManager : ObservableObject, private readonly List _builtInCommands = []; private readonly List _extensionCommandProviders = []; + private readonly Lock _commandProvidersLock = new(); TaskScheduler IPageContext.Scheduler => _taskScheduler; @@ -41,14 +42,26 @@ public partial class TopLevelCommandManager : ObservableObject, [ObservableProperty] public partial bool IsLoading { get; private set; } = true; - public IEnumerable CommandProviders => _builtInCommands.Concat(_extensionCommandProviders); + public IEnumerable CommandProviders + { + get + { + lock (_commandProvidersLock) + { + return _builtInCommands.Concat(_extensionCommandProviders).ToList(); + } + } + } public async Task LoadBuiltinsAsync() { var s = new Stopwatch(); s.Start(); - _builtInCommands.Clear(); + lock (_commandProvidersLock) + { + _builtInCommands.Clear(); + } // Load built-In commands first. These are all in-proc, and // owned by our ServiceProvider. @@ -56,7 +69,11 @@ public partial class TopLevelCommandManager : ObservableObject, foreach (var provider in builtInCommands) { CommandProviderWrapper wrapper = new(provider, _taskScheduler); - _builtInCommands.Add(wrapper); + lock (_commandProvidersLock) + { + _builtInCommands.Add(wrapper); + } + var commands = await LoadTopLevelCommandsFromProvider(wrapper); lock (TopLevelCommands) { @@ -185,6 +202,7 @@ public partial class TopLevelCommandManager : ObservableObject, IsLoading = true; var extensionService = _serviceProvider.GetService()!; await extensionService.SignalStopExtensionsAsync(); + lock (TopLevelCommands) { TopLevelCommands.Clear(); @@ -210,7 +228,11 @@ public partial class TopLevelCommandManager : ObservableObject, extensionService.OnExtensionRemoved -= ExtensionService_OnExtensionRemoved; var extensions = (await extensionService.GetInstalledExtensionsAsync()).ToImmutableList(); - _extensionCommandProviders.Clear(); + lock (_commandProvidersLock) + { + _extensionCommandProviders.Clear(); + } + if (extensions != null) { await StartExtensionsAndGetCommands(extensions); @@ -247,9 +269,9 @@ public partial class TopLevelCommandManager : ObservableObject, // Wait for all extensions to start var wrappers = (await Task.WhenAll(startTasks)).Where(wrapper => wrapper != null).Select(w => w!).ToList(); - foreach (var wrapper in wrappers) + lock (_commandProvidersLock) { - _extensionCommandProviders.Add(wrapper!); + _extensionCommandProviders.AddRange(wrappers); } // Load the commands from the providers in parallel @@ -375,4 +397,13 @@ public partial class TopLevelCommandManager : ObservableObject, var errorMessage = $"A bug occurred in {$"the \"{extensionHint}\"" ?? "an unknown's"} extension's code:\n{ex.Message}\n{ex.Source}\n{ex.StackTrace}\n\n"; CommandPaletteHost.Instance.Log(errorMessage); } + + internal bool IsProviderActive(string id) + { + lock (_commandProvidersLock) + { + return _builtInCommands.Any(wrapper => wrapper.Id == id && wrapper.IsActive) + || _extensionCommandProviders.Any(wrapper => wrapper.Id == id && wrapper.IsActive); + } + } } diff --git a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs index be245bb48a..e62fc71e2b 100644 --- a/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs +++ b/src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Apps/AllAppsCommandProvider.cs @@ -11,13 +11,15 @@ namespace Microsoft.CmdPal.Ext.Apps; public partial class AllAppsCommandProvider : CommandProvider { + public const string WellKnownId = "AllApps"; + public static readonly AllAppsPage Page = new(); private readonly CommandItem _listItem; public AllAppsCommandProvider() { - Id = "AllApps"; + Id = WellKnownId; DisplayName = Resources.installed_apps; Icon = IconHelpers.FromRelativePath("Assets\\AllApps.svg"); Settings = AllAppsSettings.Instance.Settings;