From df3e3414d21bb4d032aed5b224da07fc2ead7b11 Mon Sep 17 00:00:00 2001 From: Felipe G Date: Mon, 24 Mar 2025 10:00:34 +0000 Subject: [PATCH] Fixing command duplicates when loading commands from provider (#38091) There is a bug in command palette where the top level commands from extensions gets duplicated after the extension raises items changed, or when it is requested by CmdPal for any reason. This didn't happen when we kill CmdPal and open it again. When investigating, I noticed that the `UpdateCommandsForProvider` method was not deleting the top level commands for the extension. This seems to be happening because the comparison `var isTheSame = wrapper == firstCommand;` is not comparing objects of the same type. It was comparing a `TopLevelViewModel` with a `CommandItem`, and it was never set to `true`. I change it to compare the `TopLevelViewModel` of both commands, and now it seems to be detecting correctly. Another option of fix could be comparing the `CommandItem`s. closes https://github.com/zadjii-msft/PowerToys/issues/511 --- Another bug that I found while testing today is that when a user uninstalled or updated an extension, it was still being listed as an enabled extension. This was generating duplicates in the top level commands in case of updating an extension, and resulting in unpredictable behavior and occasional crashes in CmdPal. Added a fix for that on `ExtensionService`, removing the uninstalled extensions also from the `_enabledExtensions` list. --------- Co-authored-by: Mike Griese --- .../Models/ExtensionService.cs | 1 + .../TopLevelCommandManager.cs | 13 ++++--------- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionService.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionService.cs index 232f35c3fa..576ea08140 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionService.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionService.cs @@ -131,6 +131,7 @@ public class ExtensionService : IExtensionService, IDisposable try { _installedExtensions.RemoveAll(i => removedExtensions.Contains(i)); + _enabledExtensions.RemoveAll(i => removedExtensions.Contains(i)); OnExtensionRemoved?.Invoke(this, removedExtensions); } diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs index 1274b8800f..2b95445185 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs @@ -133,16 +133,11 @@ public partial class TopLevelCommandManager : ObservableObject, var wrapper = clone[i]; try { - // TODO! this can be safer, we're not directly exposing ICommandItem's out of CPW anymore - var thisCommand = wrapper.ItemViewModel.Model.Unsafe; - if (thisCommand != null) + var isTheSame = wrapper == firstCommand; + if (isTheSame) { - var isTheSame = thisCommand == firstCommand; - if (isTheSame) - { - startIndex = i; - break; - } + startIndex = i; + break; } } catch