CmdPal: Fix SUI crash ; Allow extensions to be disabled (#38040)

Both `TopLevelCommandItemWrapper` and `TopLevelViewModel` were really the same thing. The latter was from an earlier prototype, and the former is a more correct, safer abstraction. We really should have only ever used the former, but alas, we only used it for the SUI, and it piggy-backed off the latter, and that meant the latter's bugs became the former's.


tldr: I made the icon access safe in the SUI. 

And while I was doing this, because we now have a cleaner VM abstraction here in the host, we can actually cleanly disable extensions, because the `CommandProviderWrapper` knows which `ViewModel`s it made. 

Closes zadjii-msft#426
Closes zadjii-msft#478
Closes zadjii-msft#577

Author: zadjii-msft

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.Shell/Microsoft.CmdPal.Ext.Shell.csproj

@@ -24,9 +24,11 @@
     </EmbeddedResource>
   </ItemGroup>
 
-  
   <ItemGroup>
-    <Content Update="Assets\[email protected]">
+      <None Remove="Assets\Run_V2_2x.svg" />
+  </ItemGroup>
+  <ItemGroup>
+    <Content Update="Assets\Run_V2_2x.svg">
       <CopyToOutputDirectory>PreserveNewest</CopyToOutputDirectory>
     </Content>
   </ItemGroup>

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.Shell/Pages/ShellListPage.cs

@@ -15,7 +15,7 @@ internal sealed partial class ShellListPage : DynamicListPage
 
     public ShellListPage(SettingsManager settingsManager)
     {
-        Icon = new IconInfo("\uE756");
+        Icon = Icons.RunV2;
         Id = "com.microsoft.cmdpal.shell";
         Name = Resources.cmd_plugin_name;
         PlaceholderText = Resources.list_placeholder_text;

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.Shell/Properties/Resources.Designer.cs

@@ -130,7 +130,7 @@
     <value>execute command through command shell</value>
   </data>
   <data name="cmd_run_as_administrator" xml:space="preserve">
-    <value>Run as administrator (Ctrl+Shift+Enter)</value>
+    <value>Run as administrator</value>
   </data>
   <data name="cmd_command_failed" xml:space="preserve">
     <value>Error running the command</value>
@@ -139,7 +139,7 @@
     <value>Command not found</value>
   </data>
   <data name="cmd_run_as_user" xml:space="preserve">
-    <value>Run as different user (Ctrl+Shift+U)</value>
+    <value>Run as different user</value>
   </data>
   <data name="leave_shell_open" xml:space="preserve">
     <value>Keep shell open</value>

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.Shell/Properties/Resources.resx

@@ -18,6 +18,7 @@ public partial class SystemCommandExtensionProvider : CommandProvider
     public SystemCommandExtensionProvider()
     {
         DisplayName = Resources.Microsoft_plugin_ext_system_page_name;
+        Id = "System";
         _commands = [
             new CommandItem(Page)
             {

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.System/SystemCommandExtensionProvider.cs

@@ -22,7 +22,7 @@ public partial class TimeDateCommandsProvider : CommandProvider
     public TimeDateCommandsProvider()
     {
         DisplayName = Resources.Microsoft_plugin_timedate_plugin_name;
-
+        Id = "DateTime";
         _command = new CommandItem(_timeDateExtensionPage)
         {
             Icon = _timeDateExtensionPage.Icon,

src/modules/cmdpal/Exts/Microsoft.CmdPal.Ext.TimeDate/TimeDateCommandsProvider.cs

@@ -146,6 +146,10 @@ public override void InitializeProperties()
         UpdateProperty(nameof(Title));
         UpdateProperty(nameof(Subtitle));
         UpdateProperty(nameof(Icon));
+
+        // Load-bearing: if you don't raise a IsInitialized here, then
+        // TopLevelViewModel will never know what the command's ID is, so it
+        // will never be able to load Hotkeys & aliases
         UpdateProperty(nameof(IsInitialized));
 
         Initialized |= InitializedState.Initialized;

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs

@@ -6,6 +6,8 @@
 using Microsoft.CmdPal.Common.Services;
 using Microsoft.CmdPal.UI.ViewModels.Models;
 using Microsoft.CommandPalette.Extensions;
+using Microsoft.Extensions.DependencyInjection;
+
 using Windows.Foundation;
 
 namespace Microsoft.CmdPal.UI.ViewModels;
@@ -20,9 +22,9 @@ public sealed class CommandProviderWrapper
 
     private readonly TaskScheduler _taskScheduler;
 
-    public ICommandItem[] TopLevelItems { get; private set; } = [];
+    public TopLevelViewModel[] TopLevelItems { get; private set; } = [];
 
-    public IFallbackCommandItem[] FallbackItems { get; private set; } = [];
+    public TopLevelViewModel[] FallbackItems { get; private set; } = [];
 
     public string DisplayName { get; private set; } = string.Empty;
 
@@ -38,7 +40,13 @@ public sealed class CommandProviderWrapper
 
     public CommandSettingsViewModel? Settings { get; private set; }
 
-    public string ProviderId => $"{Extension?.PackageFamilyName ?? string.Empty}/{Id}";
+    public string ProviderId
+    {
+        get
+        {
+            return string.IsNullOrEmpty(Extension?.ExtensionUniqueId) ? Id : Extension.ExtensionUniqueId;
+        }
+    }
 
     public CommandProviderWrapper(ICommandProvider provider, TaskScheduler mainThread)
     {
@@ -105,21 +113,33 @@ public CommandProviderWrapper(IExtensionWrapper extension, TaskScheduler mainThr
         isValid = true;
     }
 
-    public async Task LoadTopLevelCommands()
+    private ProviderSettings GetProviderSettings(SettingsModel settings)
+    {
+        return settings.GetProviderSettings(this);
+    }
+
+    public async Task LoadTopLevelCommands(IServiceProvider serviceProvider, WeakReference<IPageContext> pageContext)
     {
         if (!isValid)
         {
             return;
         }
 
+        var settings = serviceProvider.GetService<SettingsModel>()!;
+
+        if (!GetProviderSettings(settings).IsEnabled)
+        {
+            return;
+        }
+
         ICommandItem[]? commands = null;
         IFallbackCommandItem[]? fallbacks = null;
 
         try
         {
             var model = _commandProvider.Unsafe!;
 
-            var t = new Task<ICommandItem[]>(model.TopLevelCommands);
+            Task<ICommandItem[]> t = new(model.TopLevelCommands);
             t.Start();
             commands = await t.ConfigureAwait(false);
 
@@ -134,6 +154,8 @@ public async Task LoadTopLevelCommands()
             Settings = new(model.Settings, this, _taskScheduler);
             Settings.InitializeProperties();
 
+            InitializeCommands(commands, fallbacks, serviceProvider, pageContext);
+
             Logger.LogDebug($"Loaded commands from {DisplayName} ({ProviderId})");
         }
         catch (Exception e)
@@ -142,15 +164,33 @@ public async Task LoadTopLevelCommands()
             Logger.LogError($"Extension was {Extension!.PackageFamilyName}");
             Logger.LogError(e.ToString());
         }
+    }
+
+    private void InitializeCommands(ICommandItem[] commands, IFallbackCommandItem[] fallbacks, IServiceProvider serviceProvider, WeakReference<IPageContext> pageContext)
+    {
+        var settings = serviceProvider.GetService<SettingsModel>()!;
+
+        Func<ICommandItem?, bool, TopLevelViewModel> makeAndAdd = (ICommandItem? i, bool fallback) =>
+        {
+            CommandItemViewModel commandItemViewModel = new(new(i), pageContext);
+            TopLevelViewModel topLevelViewModel = new(commandItemViewModel, fallback, ExtensionHost, ProviderId, settings, serviceProvider);
+
+            topLevelViewModel.ItemViewModel.SlowInitializeProperties();
 
+            return topLevelViewModel;
+        };
         if (commands != null)
         {
-            TopLevelItems = commands;
+            TopLevelItems = commands
+                .Select(c => makeAndAdd(c, false))
+                .ToArray();
         }
 
         if (fallbacks != null)
         {
-            FallbackItems = fallbacks;
+            FallbackItems = fallbacks
+                .Select(c => makeAndAdd(c, true))
+                .ToArray();
         }
     }
 

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs

@@ -49,6 +49,7 @@ public void FastInitializeProperties()
             return;
         }
 
+        Id = model.Id ?? string.Empty;
         Name = model.Name ?? string.Empty;
         IsFastInitialized = true;
     }

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandViewModel.cs

@@ -73,7 +73,6 @@ public override IListItem[] GetItems()
             {
                 return _tlcManager
                     .TopLevelCommands
-                    .Select(tlc => tlc)
                     .Where(tlc => !string.IsNullOrEmpty(tlc.Title))
                     .ToArray();
             }
@@ -167,16 +166,17 @@ private int ScoreTopLevelItem(string query, IListItem topLevelOrAppItem)
         var id = IdForTopLevelOrAppItem(topLevelOrAppItem);
 
         var extensionDisplayName = string.Empty;
-        if (topLevelOrAppItem is TopLevelCommandItemWrapper toplevel)
+        if (topLevelOrAppItem is TopLevelViewModel topLevel)
         {
-            isFallback = toplevel.IsFallback;
-            if (toplevel.Alias?.Alias is string alias)
+            isFallback = topLevel.IsFallback;
+            if (topLevel.HasAlias)
             {
+                var alias = topLevel.AliasText;
                 isAliasMatch = alias == query;
                 isAliasSubstringMatch = isAliasMatch || alias.StartsWith(query, StringComparison.CurrentCultureIgnoreCase);
             }
 
-            extensionDisplayName = toplevel.ExtensionHost?.Extension?.PackageDisplayName ?? string.Empty;
+            extensionDisplayName = topLevel.ExtensionHost?.Extension?.PackageDisplayName ?? string.Empty;
         }
 
         var nameMatch = StringMatcher.FuzzySearch(query, title);
@@ -221,9 +221,9 @@ public void UpdateHistory(IListItem topLevelOrAppItem)
 
     private string IdForTopLevelOrAppItem(IListItem topLevelOrAppItem)
     {
-        if (topLevelOrAppItem is TopLevelCommandItemWrapper toplevel)
+        if (topLevelOrAppItem is TopLevelViewModel topLevel)
         {
-            return toplevel.Id;
+            return topLevel.Id;
         }
         else
         {

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs

@@ -9,7 +9,7 @@
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class IconDataViewModel : ObservableObject
+public partial class IconDataViewModel : ObservableObject, IIconData
 {
     private readonly ExtensionObject<IIconData> _model = new(null);
 
@@ -25,6 +25,8 @@ public partial class IconDataViewModel : ObservableObject
     // first. Hence why we're sticking this into an ExtensionObject
     public ExtensionObject<IRandomAccessStreamReference> Data { get; private set; } = new(null);
 
+    IRandomAccessStreamReference? IIconData.Data => Data.Unsafe;
+
     public IconDataViewModel(IIconData? icon)
     {
         _model = new(icon);

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/IconDataViewModel.cs

@@ -8,7 +8,7 @@
 
 namespace Microsoft.CmdPal.UI.ViewModels;
 
-public partial class IconInfoViewModel : ObservableObject
+public partial class IconInfoViewModel : ObservableObject, IIconInfo
 {
     private readonly ExtensionObject<IIconInfo> _model = new(null);
 
@@ -28,6 +28,10 @@ public partial class IconInfoViewModel : ObservableObject
 
     public bool IsSet => _model.Unsafe != null;
 
+    IIconData? IIconInfo.Dark => Dark;
+
+    IIconData? IIconInfo.Light => Light;
+
     public IconInfoViewModel(IIconInfo? icon)
     {
         _model = new(icon);

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/IconInfoViewModel.cs

@@ -18,16 +18,19 @@ public record PerformCommandMessage
 
     public bool WithAnimation { get; set; } = true;
 
+    public CommandPaletteHost? ExtensionHost { get; private set; }
+
     public PerformCommandMessage(ExtensionObject<ICommand> command)
     {
         Command = command;
         Context = null;
     }
 
-    public PerformCommandMessage(TopLevelCommandItemWrapper topLevelCommand)
+    public PerformCommandMessage(TopLevelViewModel topLevelCommand)
     {
-        Command = new(topLevelCommand.Command);
+        Command = topLevelCommand.CommandViewModel.Model;
         Context = null;
+        ExtensionHost = topLevelCommand.ExtensionHost;
     }
 
     public PerformCommandMessage(ExtensionObject<ICommand> command, ExtensionObject<IListItem> context)

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Messages/PerformCommandMessage.cs

@@ -106,14 +106,14 @@ await Task.Run(() =>
                 {
                     Logger.LogDebug($"Starting {ExtensionDisplayName} ({ExtensionClassId})");
 
-                    nint extensionPtr = nint.Zero;
+                    var extensionPtr = nint.Zero;
                     try
                     {
                         // -2147024809: E_INVALIDARG
                         // -2147467262: E_NOINTERFACE
                         // -2147024893: E_PATH_NOT_FOUND
-                        Guid guid = typeof(IExtension).GUID;
-                        global::Windows.Win32.Foundation.HRESULT hr = PInvoke.CoCreateInstance(Guid.Parse(ExtensionClassId), null, CLSCTX.CLSCTX_LOCAL_SERVER, guid, out object? extensionObj);
+                        var guid = typeof(IExtension).GUID;
+                        var hr = PInvoke.CoCreateInstance(Guid.Parse(ExtensionClassId), null, CLSCTX.CLSCTX_LOCAL_SERVER, guid, out var extensionObj);
 
                         if (hr.Value == -2147024893)
                         {
@@ -179,7 +179,7 @@ public async Task<IEnumerable<T>> GetListOfProvidersAsync<T>()
     {
         await StartExtensionAsync();
 
-        object? supportedProviders = GetExtensionObject()?.GetProvider(_providerTypeMap[typeof(T)]);
+        var supportedProviders = GetExtensionObject()?.GetProvider(_providerTypeMap[typeof(T)]);
         if (supportedProviders is IEnumerable<T> multipleProvidersSupported)
         {
             return multipleProvidersSupported;

src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Models/ExtensionWrapper.cs

@@ -236,4 +236,7 @@
   <data name="builtin_reload_display_title" xml:space="preserve">
     <value>Reload Command Palette extensions</value>
   </data>
+  <data name="builtin_disabled_extension" xml:space="preserve">
+    <value>Disabled</value>
+  </data>
 </root>