From b0bfeec8887cabc033d53765d03060592dd01a41 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 16 Dec 2024 15:49:57 -0600 Subject: [PATCH] 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 --- .../CommandItemViewModel.cs | 1 - .../CommandProviderWrapper.cs | 9 +++ .../Commands/MainListPage.cs | 7 ++- .../Commands/QuitCommandProvider.cs | 5 +- .../TopLevelCommandManager.cs | 7 ++- .../TopLevelCommandWrapper.cs | 63 ++++++++++++++++++- .../ListHelpers.cs | 5 ++ .../FallbackExecuteItem.cs | 9 +-- 8 files changed, 91 insertions(+), 15 deletions(-) diff --git a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs index d09281b5a6..6deaa06166 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/CommandItemViewModel.cs @@ -133,7 +133,6 @@ public partial class CommandItemViewModel : ExtensionObjectViewModel 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 index d7ced23063..63259cb1de 100644 --- 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 sealed class CommandProviderWrapper 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 index 1e0a6f2fb0..049ff241a0 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/Commands/MainListPage.cs @@ -59,7 +59,7 @@ public partial class MainListPage : DynamicListPage } return string.IsNullOrEmpty(SearchText) - ? _commands.Select(tlc => tlc).ToArray() + ? _commands.Select(tlc => tlc).Where(tlc => !string.IsNullOrEmpty(tlc.Title)).ToArray() : _filteredItems?.ToArray() ?? []; } @@ -67,6 +67,11 @@ public partial class MainListPage : DynamicListPage { /* 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 index e0473b0d09..97be41f60b 100644 --- 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 index b8d2a3bddd..c8543b8232 100644 --- a/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs +++ b/src/modules/cmdpal/Microsoft.CmdPal.UI.ViewModels/TopLevelCommandManager.cs @@ -40,7 +40,12 @@ public partial class TopLevelCommandManager(IServiceProvider _serviceProvider) : 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 index 8a04189d42..125cb04229 100644 --- 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 Model { get; } - public TopLevelCommandWrapper(ExtensionObject commandItem) + private readonly bool _isFallback; + + public TopLevelCommandWrapper(ExtensionObject 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 FromExtension(ExtensionObject)` @@ -38,10 +42,67 @@ public partial class TopLevelCommandWrapper : ListItem 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 index 4db3199762..5adc75dcf0 100644 --- 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 class ListHelpers 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 index 46fc0dd7cf..321e60bad9 100644 --- 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 @@ internal sealed partial class FallbackExecuteItem : FallbackCommandItem { _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; } }