Make fallback commands work again (#234)

Add support for fallback commands again. Couple important parts: * Lists shouldn't contain items with blank `Title`s. That's spec'd that way, and allows fallback items to hide themselves if they don't want to be shown for a given search string * The CommandProviderWrapper, as well as extension infrastructure needs to know to get the fallbacks out of a commandprovider too. * `TopLevelCommandWrapper` needs to know when its model changes, to update itself in the list ------ Co-authored-by: Mike Griese <[email protected]>
zadjii-msft · Dec 16, 2024 · b0bfeec · b0bfeec
1 parent 4048d76
commit b0bfeec
Show file tree

Hide file tree

Showing 8 changed files with 91 additions and 15 deletions.
diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs
@@ -133,7 +133,6 @@ protected virtual void FetchProperty(string propertyName)
                 this.Icon = model.Icon;
                 break;
 
-                // TODO! Icon
                 // TODO! MoreCommands array, which needs to also raise HasMoreCommands
         }
 

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandProviderWrapper.cs
@@ -19,6 +19,8 @@ public sealed class CommandProviderWrapper
 
     public ICommandItem[] TopLevelItems { get; private set; } = [];
 
+    public IFallbackCommandItem[] FallbackItems { get; private set; } = [];
+
     public CommandProviderWrapper(ICommandProvider provider)
     {
         _commandProvider = provider;
@@ -56,10 +58,17 @@ public async Task LoadTopLevelCommands()
         var commands = await t.ConfigureAwait(false);
 
         // On a BG thread here
+        var fallbacks = _commandProvider.FallbackCommands();
+
         if (commands != null)
         {
             TopLevelItems = commands;
         }
+
+        if (fallbacks != null)
+        {
+            FallbackItems = fallbacks;
+        }
     }
 
     /* This is a View/ExtensionHost piece

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs
@@ -59,14 +59,19 @@ public override IListItem[] GetItems()
         }
 
         return string.IsNullOrEmpty(SearchText)
-            ? _commands.Select(tlc => tlc).ToArray()
+            ? _commands.Select(tlc => tlc).Where(tlc => !string.IsNullOrEmpty(tlc.Title)).ToArray()
             : _filteredItems?.ToArray() ?? [];
     }
 
     public override void UpdateSearchText(string oldSearch, string newSearch)
     {
         /* handle changes to the filter text here */
 
+        foreach (var command in _commands)
+        {
+            command.TryUpdateFallbackText(newSearch);
+        }
+
         // Cleared out the filter text? easy. Reset _filteredItems, and bail out.
         if (string.IsNullOrEmpty(newSearch))
         {

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/QuitCommandProvider.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/QuitCommandProvider.cs
@@ -15,10 +15,7 @@ public partial class QuitCommandProvider : CommandProvider
 {
     private readonly QuitAction quitAction = new();
 
-    public override ICommandItem[] TopLevelCommands() =>
-
-        // HACK: fallback commands aren't wired up and we need to be able to exit
-        [new FallbackCommandItem(quitAction) { Subtitle = "Exit Command Palette" }];
+    public override ICommandItem[] TopLevelCommands() => [];
 
     public override IFallbackCommandItem[] FallbackCommands() =>
         [new FallbackCommandItem(quitAction) { Subtitle = "Exit Command Palette" }];

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs
@@ -40,7 +40,12 @@ private async Task LoadTopLevelCommandsFromProvider(CommandProviderWrapper comma
         await commandProvider.LoadTopLevelCommands();
         foreach (var i in commandProvider.TopLevelItems)
         {
-            TopLevelCommands.Add(new(new(i)));
+            TopLevelCommands.Add(new(new(i), false));
+        }
+
+        foreach (var i in commandProvider.FallbackItems)
+        {
+            TopLevelCommands.Add(new(new(i), true));
         }
     }
 

diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandWrapper.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandWrapper.cs
@@ -17,9 +17,13 @@ public partial class TopLevelCommandWrapper : ListItem
 {
     public ExtensionObject<ICommandItem> Model { get; }
 
-    public TopLevelCommandWrapper(ExtensionObject<ICommandItem> commandItem)
+    private readonly bool _isFallback;
+
+    public TopLevelCommandWrapper(ExtensionObject<ICommandItem> commandItem, bool isFallback)
         : base(commandItem.Unsafe?.Command ?? new NoOpCommand())
     {
+        _isFallback = isFallback;
+
         // TODO: In reality, we should do an async fetch when we're created
         // from an extension object. Probably have an
         // `static async Task<TopLevelCommandWrapper> FromExtension(ExtensionObject<ICommandItem>)`
@@ -38,10 +42,67 @@ public TopLevelCommandWrapper(ExtensionObject<ICommandItem> commandItem)
             Subtitle = model.Subtitle;
             Icon = new(model.Icon.Icon);
             MoreCommands = model.MoreCommands;
+
+            model.PropChanged += Model_PropChanged;
         }
         catch (Exception ex)
         {
             System.Diagnostics.Debug.WriteLine(ex);
         }
     }
+
+    private void Model_PropChanged(object sender, PropChangedEventArgs args)
+    {
+        try
+        {
+            var propertyName = args.PropertyName;
+            var model = Model.Unsafe;
+            if (model == null)
+            {
+                return; // throw?
+            }
+
+            switch (propertyName)
+            {
+                case nameof(Title):
+                    this.Title = model.Title;
+                    break;
+                case nameof(Subtitle):
+                    this.Subtitle = model.Subtitle;
+                    break;
+                case nameof(Icon):
+                    var listIcon = model.Icon;
+                    Icon = model.Icon;
+                    break;
+
+                    // TODO! MoreCommands array, which needs to also raise HasMoreCommands
+            }
+        }
+        catch
+        {
+        }
+    }
+
+    public void TryUpdateFallbackText(string newQuery)
+    {
+        if (!_isFallback)
+        {
+            return;
+        }
+
+        try
+        {
+            _ = Task.Run(() =>
+            {
+                var model = Model.Unsafe;
+                if (model is IFallbackCommandItem fallback)
+                {
+                    fallback.FallbackHandler.UpdateQuery(newQuery);
+                }
+            });
+        }
+        catch (Exception)
+        {
+        }
+    }
 }
diff --git a/src/modules/cmdpal/extensionsdk/Microsoft.CmdPal.Extensions.Helpers/ListHelpers.cs b/src/modules/cmdpal/extensionsdk/Microsoft.CmdPal.Extensions.Helpers/ListHelpers.cs
@@ -16,6 +16,11 @@ public static int ScoreListItem(string query, ICommandItem listItem)
             return 1;
         }
 
+        if (string.IsNullOrEmpty(listItem.Title))
+        {
+            return 0;
+        }
+
         var nameMatch = StringMatcher.FuzzySearch(query, listItem.Title);
 
         // var locNameMatch = StringMatcher.FuzzySearch(query, NameLocalized);

diff --git a/src/modules/cmdpal/exts/Microsoft.CmdPal.Ext.Shell/FallbackExecuteItem.cs b/src/modules/cmdpal/exts/Microsoft.CmdPal.Ext.Shell/FallbackExecuteItem.cs
@@ -4,9 +4,6 @@
 
 using Microsoft.CmdPal.Ext.Shell.Commands;
 using Microsoft.CmdPal.Ext.Shell.Helpers;
-using Microsoft.CmdPal.Ext.Shell.Pages;
-using Microsoft.CmdPal.Ext.Shell.Properties;
-using Microsoft.CmdPal.Extensions;
 using Microsoft.CmdPal.Extensions.Helpers;
 
 namespace Microsoft.CmdPal.Ext.Shell;
@@ -20,17 +17,15 @@ public FallbackExecuteItem(SettingsManager settings)
     {
         _executeItem = (ExecuteItem)this.Command!;
         Title = string.Empty;
+        _executeItem.Name = string.Empty;
         Subtitle = Properties.Resources.generic_run_command;
         Icon = new("\uE756");
-
-        // TODO: this is a bug in the current POC. I don't think Fallback items
-        // get icons set correctly.
-        _executeItem.Icon = Icon;
     }
 
     public override void UpdateQuery(string query)
     {
         _executeItem.Cmd = query;
+        _executeItem.Name = string.IsNullOrEmpty(query) ? string.Empty : Properties.Resources.generic_run_command;
         Title = query;
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -133,7 +133,6 @@ protected virtual void FetchProperty(string propertyName) @@
                     this.Icon = model.Icon;
                     break;
-                    // TODO! Icon
                     // TODO! MoreCommands array, which needs to also raise HasMoreCommands
             }
@@ Expand Down @@