cmdpal: split TLVM into a subclass TopLevelFallbackViewModel (#51527)

Sensible refactoring ahead of fallbacksv2 support.

TLVM has a lot of gross code that's all like
`if(IsFallback) { doFallbackThing(); } else { doNormal(); }`. Well
that's OOP 101 folks. Don't do that.

This creates a proper TopLevel**Fallback**ViewModel class, which
inherits from TopLevelViewModel class. That way we can encapsulate all
the fallback code into one sensible class.

Derived from work in 7eab9b4b75...3c902c158e

Closes nothing, I work here

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

@@ -458,7 +458,9 @@ public sealed class CommandProviderWrapper : ICommandProviderContext
         TopLevelViewModel? topLevelViewModel = null;
         try
         {
-            topLevelViewModel = new(commandItemViewModel, type, ExtensionHost, providerContext, providerSettings, serviceProvider, item, contextMenuFactory);
+            topLevelViewModel = type == TopLevelType.Fallback
+                ? new TopLevelFallbackViewModel(commandItemViewModel, ExtensionHost, providerContext, providerSettings, serviceProvider, item as IFallbackCommandItem, contextMenuFactory)
+                : new TopLevelViewModel(commandItemViewModel, type, ExtensionHost, providerContext, providerSettings, serviceProvider, contextMenuFactory);
             topLevelViewModel.InitializeProperties();
             return topLevelViewModel;
         }

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

@@ -0,0 +1,105 @@
+// Copyright (c) Microsoft Corporation
+// The Microsoft Corporation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using ManagedCommon;
+using Microsoft.CmdPal.UI.ViewModels.Models;
+using Microsoft.CommandPalette.Extensions;
+
+namespace Microsoft.CmdPal.UI.ViewModels;
+
+/// <summary>
+/// A <see cref="TopLevelViewModel"/> for a fallback command item. Holds the
+/// fallback-specific state (fallback ID, display title, per-fallback enabled
+/// setting) and forwards query updates to the extension's fallback handler.
+/// </summary>
+internal sealed partial class TopLevelFallbackViewModel : TopLevelViewModel
+{
+    private readonly ExtensionObject<IFallbackCommandItem> _model;
+    private readonly string _fallbackId = string.Empty;
+    private string _displayTitle = string.Empty;
+
+    protected override string IdFromModel => string.IsNullOrWhiteSpace(_fallbackId) ? base.IdFromModel : _fallbackId;
+
+    public override bool IsFallback => true;
+
+    public override string DisplayTitle => _displayTitle;
+
+    public override bool IsEnabled => !ProviderSettings.FallbackCommands.TryGetValue(_fallbackId, out var settings) || settings.IsEnabled;
+
+    internal TopLevelFallbackViewModel(
+        CommandItemViewModel item,
+        CommandPaletteHost extensionHost,
+        ICommandProviderContext commandProviderContext,
+        ProviderSettings providerSettings,
+        IServiceProvider serviceProvider,
+        IFallbackCommandItem? model,
+        IContextMenuFactory? contextMenuFactory)
+        : base(item, TopLevelType.Fallback, extensionHost, commandProviderContext, providerSettings, serviceProvider, contextMenuFactory)
+    {
+        _model = new(model);
+
+        // RPC to check type
+        if (model is IFallbackCommandItem2 fallback2)
+        {
+            _fallbackId = fallback2.Id;
+        }
+    }
+
+    internal override void InitializeProperties()
+    {
+        base.InitializeProperties();
+
+        // RPC
+        if (_model.Unsafe is { } model)
+        {
+            _displayTitle = model.DisplayTitle;
+        }
+
+        UpdateInitialIcon(false);
+    }
+
+    internal override bool SafeUpdateFallbackTextSynchronous(string newQuery)
+    {
+        if (!IsEnabled)
+        {
+            return false;
+        }
+
+        try
+        {
+            return UnsafeUpdateFallbackSynchronous(newQuery);
+        }
+        catch (Exception ex)
+        {
+            Logger.LogError(ex.ToString());
+        }
+
+        return false;
+    }
+
+    /// <summary>
+    /// Calls UpdateQuery on our fallback handler. This does RPC work, so make
+    /// sure you're calling it on a BG thread.
+    /// </summary>
+    /// <param name="newQuery">The new search text to pass to the extension</param>
+    /// <returns>true if our Title changed across this call</returns>
+    private bool UnsafeUpdateFallbackSynchronous(string newQuery)
+    {
+        if (_model.Unsafe is not { } fallback)
+        {
+            return false;
+        }
+
+        var oldTitle = Title;
+
+        // RPC for method
+        fallback.FallbackHandler.UpdateQuery(newQuery);
+        var newTitle = Title;
+
+        // Report any title change, not just an empty <-> non-empty flip: the render path
+        // re-scores fallbacks off this signal, so a change like "server01" -> "server02"
+        // must still trigger a refresh or the fallback keeps its stale score and position.
+        return !string.Equals(oldTitle, newTitle, StringComparison.Ordinal);
+    }
+}

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

@@ -5,7 +5,6 @@
 using System.Collections.ObjectModel;
 using System.Diagnostics;
 using CommunityToolkit.Mvvm.ComponentModel;
-using ManagedCommon;
 using Microsoft.CmdPal.Common.Helpers;
 using Microsoft.CmdPal.Common.Text;
 using Microsoft.CmdPal.UI.ViewModels.Messages;
@@ -20,10 +19,9 @@ using WyHash;
 namespace Microsoft.CmdPal.UI.ViewModels;
 
 [DebuggerDisplay($"{{{nameof(GetDebuggerDisplay)}(),nq}}")]
-public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IExtendedAttributesProvider, IPrecomputedListItem, ICommandContextSource
+public partial class TopLevelViewModel : ObservableObject, IListItem, IExtendedAttributesProvider, IPrecomputedListItem, ICommandContextSource
 {
     private readonly ISettingsService _settingsService;
-    private readonly ProviderSettings _providerSettings;
     private readonly IServiceProvider _serviceProvider;
     private readonly CommandItemViewModel _commandItemViewModel;
     private readonly IContextMenuFactory _contextMenuFactory;
@@ -31,9 +29,9 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
 
     public ICommandProviderContext ProviderContext { get; private set; }
 
-    private string IdFromModel => IsFallback && !string.IsNullOrWhiteSpace(_fallbackId) ? _fallbackId : _commandItemViewModel.Command.Id;
+    protected ProviderSettings ProviderSettings { get; }
 
-    private string _fallbackId = string.Empty;
+    protected virtual string IdFromModel => _commandItemViewModel.Command.Id;
 
     private string _generatedId = string.Empty;
 
@@ -46,7 +44,7 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
 
     private CommandAlias? Alias { get; set; }
 
-    public bool IsFallback { get; private set; }
+    public virtual bool IsFallback => false;
 
     [ObservableProperty]
     public partial ObservableCollection<Tag> Tags { get; set; } = [];
@@ -93,7 +91,7 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
     public event TypedEventHandler<object, IPropChangedEventArgs>? PropChanged;
 
     // Fallback items
-    public string DisplayTitle { get; private set; } = string.Empty;
+    public virtual string DisplayTitle => string.Empty;
 
     public HotkeySettings? Hotkey
     {
@@ -157,25 +155,7 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
         }
     }
 
-    public bool IsEnabled
-    {
-        get
-        {
-            if (IsFallback)
-            {
-                if (_providerSettings.FallbackCommands.TryGetValue(_fallbackId, out var fallbackSettings))
-                {
-                    return fallbackSettings.IsEnabled;
-                }
-
-                return true;
-            }
-            else
-            {
-                return _providerSettings.IsEnabled;
-            }
-        }
-    }
+    public virtual bool IsEnabled => ProviderSettings.IsEnabled;
 
     // Dock properties
     public bool IsDockBand { get; private set; }
@@ -214,30 +194,24 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
         ICommandProviderContext commandProviderContext,
         ProviderSettings providerSettings,
         IServiceProvider serviceProvider,
-        ICommandItem? commandItem,
         IContextMenuFactory? contextMenuFactory)
     {
         _serviceProvider = serviceProvider;
         _settingsService = serviceProvider.GetRequiredService<ISettingsService>();
-        _providerSettings = providerSettings;
+        ProviderSettings = providerSettings;
         ProviderContext = commandProviderContext;
         _commandItemViewModel = item;
         _contextItemsVersion = item.ContextItemsVersion;
 
         _contextMenuFactory = contextMenuFactory ?? DefaultContextMenuFactory.Instance;
 
-        IsFallback = topLevelType == TopLevelType.Fallback;
         IsDockBand = topLevelType == TopLevelType.DockBand;
         ExtensionHost = extensionHost;
-        if (IsFallback && commandItem is IFallbackCommandItem2 fallback)
-        {
-            _fallbackId = fallback.Id;
-        }
 
         item.PropertyChangedBackground += Item_PropertyChanged;
     }
 
-    internal void InitializeProperties()
+    internal virtual void InitializeProperties()
     {
         // Init first, so that we get the ID & titles,
         // then generate the ID,
@@ -245,19 +219,6 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
         ItemViewModel.InitializeProperties();
         GenerateId();
         ItemViewModel.SlowInitializeProperties();
-
-        if (IsFallback)
-        {
-            var model = _commandItemViewModel.Model.Unsafe;
-
-            // RPC to check type
-            if (model is IFallbackCommandItem fallback)
-            {
-                DisplayTitle = fallback.DisplayTitle;
-            }
-
-            UpdateInitialIcon(false);
-        }
     }
 
     private void Item_PropertyChanged(object? sender, System.ComponentModel.PropertyChangedEventArgs e)
@@ -312,7 +273,7 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
         }
     }
 
-    private void UpdateInitialIcon(bool raiseNotification = true)
+    protected void UpdateInitialIcon(bool raiseNotification = true)
     {
         if (_initialIcon != null || !_commandItemViewModel.Icon.IsSet)
         {
@@ -414,57 +375,7 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
         }
     }
 
-    internal bool SafeUpdateFallbackTextSynchronous(string newQuery)
-    {
-        if (!IsFallback)
-        {
-            return false;
-        }
-
-        if (!IsEnabled)
-        {
-            return false;
-        }
-
-        try
-        {
-            return UnsafeUpdateFallbackSynchronous(newQuery);
-        }
-        catch (Exception ex)
-        {
-            Logger.LogError(ex.ToString());
-        }
-
-        return false;
-    }
-
-    /// <summary>
-    /// Calls UpdateQuery on our command, if we're a fallback item. This does
-    /// RPC work, so make sure you're calling it on a BG thread.
-    /// </summary>
-    /// <param name="newQuery">The new search text to pass to the extension</param>
-    /// <returns>true if our Title changed across this call</returns>
-    private bool UnsafeUpdateFallbackSynchronous(string newQuery)
-    {
-        var model = _commandItemViewModel.Model.Unsafe;
-
-        // RPC to check type
-        if (model is IFallbackCommandItem fallback)
-        {
-            var oldTitle = Title;
-
-            // RPC for method
-            fallback.FallbackHandler.UpdateQuery(newQuery);
-            var newTitle = Title;
-
-            // Report any title change, not just an empty <-> non-empty flip: the render path
-            // re-scores fallbacks off this signal, so a change like "server01" -> "server02"
-            // must still trigger a refresh or the fallback keeps its stale score and position.
-            return !string.Equals(oldTitle, newTitle, StringComparison.Ordinal);
-        }
-
-        return false;
-    }
+    internal virtual bool SafeUpdateFallbackTextSynchronous(string newQuery) => false;
 
     public PerformCommandMessage GetPerformCommandMessage()
     {
@@ -523,7 +434,7 @@ public sealed partial class TopLevelViewModel : ObservableObject, IListItem, IEx
     /// and cleans up its resources so the TopLevelViewModel can be garbage
     /// collected after it is removed from the owning collections.
     /// </summary>
-    internal void Cleanup()
+    internal virtual void Cleanup()
     {
         _commandItemViewModel.PropertyChangedBackground -= Item_PropertyChanged;
         _commandItemViewModel.SafeCleanup();

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandContextSourceTests.cs

@@ -43,7 +43,7 @@ public sealed class CommandContextSourceTests
         var model = new CommandItem(page);
         var itemViewModel = new CommandItemViewModel(new(model), new WeakReference<IPageContext>(pageContext), DefaultContextMenuFactory.Instance);
         itemViewModel.SlowInitializeProperties();
-        var topLevel = new TopLevelViewModel(itemViewModel, TopLevelType.Normal, host, provider, new ProviderSettings(), services, model, DefaultContextMenuFactory.Instance);
+        var topLevel = new TopLevelViewModel(itemViewModel, TopLevelType.Normal, host, provider, new ProviderSettings(), services, DefaultContextMenuFactory.Instance);
 
         try
         {

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/CommandItemViewModelTests.cs

@@ -523,7 +523,7 @@ public partial class CommandItemViewModelTests
         await uiTasks.StartNew(() => { });
 
         using var services = new ServiceCollection().AddSingleton(Mock.Of<ISettingsService>()).BuildServiceProvider();
-        var adapter = new TopLevelViewModel(viewModel, TopLevelType.Normal, CommandPaletteHost.Instance, CommandProviderContext.Empty, new(), services, item, DefaultContextMenuFactory.Instance);
+        var adapter = new TopLevelViewModel(viewModel, TopLevelType.Normal, CommandPaletteHost.Instance, CommandProviderContext.Empty, new(), services, DefaultContextMenuFactory.Instance);
         var wrappedViewModel = new CommandItemViewModel(new(adapter), new(pageContext), DefaultContextMenuFactory.Instance);
         wrappedViewModel.SlowInitializeProperties();
         var wrappedNotified = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously);
@@ -1395,7 +1395,7 @@ public partial class CommandItemViewModelTests
         await uiTasks.StartNew(() => { });
 
         using var services = new ServiceCollection().AddSingleton(Mock.Of<ISettingsService>()).BuildServiceProvider();
-        var adapter = new TopLevelViewModel(source, TopLevelType.Normal, CommandPaletteHost.Instance, CommandProviderContext.Empty, new(), services, item, DefaultContextMenuFactory.Instance);
+        var adapter = new TopLevelViewModel(source, TopLevelType.Normal, CommandPaletteHost.Instance, CommandProviderContext.Empty, new(), services, DefaultContextMenuFactory.Instance);
         var wrapped = new ListItemViewModel(adapter, new(pageContext), DefaultContextMenuFactory.Instance);
         wrapped.SlowInitializeProperties();
         var secondary = (CommandContextItemViewModel)wrapped.SecondaryCommand!;

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DockBandsCollectionTests.cs

@@ -115,7 +115,6 @@ public sealed partial class DockBandsCollectionTests
             CommandProviderContext.Empty,
             new ProviderSettings(),
             serviceProvider,
-            item,
             DefaultContextMenuFactory.Instance);
     }
 }

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/DockItemViewModel.PinningTests.cs

@@ -279,7 +279,6 @@ public partial class DockItemViewModelTests
             context.ProviderContext,
             new ProviderSettings(),
             services.Object,
-            item,
             DefaultContextMenuFactory.Instance);
         topLevel.InitializeProperties();
 

src/modules/cmdpal/Tests/Microsoft.CmdPal.UI.ViewModels.UnitTests/MainListPageCacheTests.cs

@@ -235,7 +235,6 @@ public sealed class MainListPageCacheTests
             providerContext,
             new ProviderSettings(),
             services,
-            model,
             DefaultContextMenuFactory.Instance);
         return topLevel;
     }