Give Tab ownership of its SwitchToTab command (#7659)

Currently, `CommandPalette` creates and maintains the `SwitchToTab`
commands used for the ATS. When `Command` goes into the
TerminalSettingsModel, the palette won't be able to access `Command`'s
implementation type, making it difficult for `CommandPalette` to tell
`Command` to listen to `Tab` for changes.

This PR changes the relationship up so `Tab` now manages its
`SwitchToTab` command, and `CommandPalette` just plops the command from
`Tab` into its list.

(cherry picked from commit 468c8c6)

src/cascadia/TerminalApp/CommandPalette.cpp

@@ -1067,30 +1067,17 @@ namespace winrt::TerminalApp::implementation
             {
             case CollectionChange::ItemChanged:
             {
-                winrt::com_ptr<Command> item;
-                item.copy_from(winrt::get_self<Command>(_allTabActions.GetAt(idx)));
-                item->propertyChangedRevoker.revoke();
-
-                auto tab = tabList.GetAt(idx);
-                GenerateCommandForTab(idx, false, tab);
-                UpdateTabIndices(idx);
                 break;
             }
             case CollectionChange::ItemInserted:
             {
                 auto tab = tabList.GetAt(idx);
-                GenerateCommandForTab(idx, true, tab);
-                UpdateTabIndices(idx);
+                _allTabActions.InsertAt(idx, tab.SwitchToTabCommand());
                 break;
             }
             case CollectionChange::ItemRemoved:
             {
-                winrt::com_ptr<Command> item;
-                item.copy_from(winrt::get_self<Command>(_allTabActions.GetAt(idx)));
-                item->propertyChangedRevoker.revoke();
-
                 _allTabActions.RemoveAt(idx);
-                UpdateTabIndices(idx);
                 break;
             }
             }
@@ -1099,83 +1086,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    // Method Description:
-    // - In the case where a tab is removed or reordered, the given indices of
-    //   the tab switch commands following the removed/reordered tab will get out of sync by 1
-    //   (e.g. if tab 1 is removed, tabs 2,3,4,... need to become tabs 1,2,3,...)
-    //   This function just loops through the tabs following startIdx and adjusts their given indices.
-    // Arguments:
-    // - startIdx: The index to start the update loop at.
-    // Return Value:
-    // - <none>
-    void CommandPalette::UpdateTabIndices(const uint32_t startIdx)
-    {
-        for (auto i = startIdx; i < _allTabActions.Size(); ++i)
-        {
-            auto command = _allTabActions.GetAt(i);
-
-            command.Action().Args().as<implementation::SwitchToTabArgs>()->TabIndex(i);
-        }
-    }
-
-    // Method Description:
-    // - Create a tab switching command based on the given tab object and insert/update the command
-    //   at the given index. The command will call a SwitchToTab action on the given idx.
-    // Arguments:
-    // - idx: The index to insert or update the tab switch command.
-    // - tab: The tab object to refer to when creating the tab switch command.
-    // Return Value:
-    // - <none>
-    void CommandPalette::GenerateCommandForTab(const uint32_t idx, bool inserted, TerminalApp::Tab& tab)
-    {
-        auto focusTabAction = winrt::make_self<implementation::ActionAndArgs>();
-        auto args = winrt::make_self<implementation::SwitchToTabArgs>();
-        args->TabIndex(idx);
-
-        focusTabAction->Action(ShortcutAction::SwitchToTab);
-        focusTabAction->Args(*args);
-
-        auto command = winrt::make_self<implementation::Command>();
-        command->Action(*focusTabAction);
-        command->Name(tab.Title());
-        command->IconSource(tab.IconSource());
-
-        // Listen for changes to the Tab so we can update this Command's attributes accordingly.
-        auto weakThis{ get_weak() };
-        auto weakCommand{ command->get_weak() };
-        command->propertyChangedRevoker = tab.PropertyChanged(winrt::auto_revoke, [weakThis, weakCommand, tab](auto&&, const Windows::UI::Xaml::Data::PropertyChangedEventArgs& args) {
-            auto palette{ weakThis.get() };
-            auto command{ weakCommand.get() };
-
-            if (palette && command)
-            {
-                if (args.PropertyName() == L"Title")
-                {
-                    if (command->Name() != tab.Title())
-                    {
-                        command->Name(tab.Title());
-                    }
-                }
-                if (args.PropertyName() == L"IconSource")
-                {
-                    if (command->IconSource() != tab.IconSource())
-                    {
-                        command->IconSource(tab.IconSource());
-                    }
-                }
-            }
-        });
-
-        if (inserted)
-        {
-            _allTabActions.InsertAt(idx, *command);
-        }
-        else
-        {
-            _allTabActions.SetAt(idx, *command);
-        }
-    }
-
     void CommandPalette::EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx)
     {
         _switcherStartIdx = startIdx;

src/cascadia/TerminalApp/CommandPalette.h

@@ -94,8 +94,6 @@ namespace winrt::TerminalApp::implementation
         Microsoft::Terminal::TerminalControl::IKeyBindings _bindings;
 
         // Tab Switcher
-        void GenerateCommandForTab(const uint32_t idx, bool inserted, winrt::TerminalApp::Tab& tab);
-        void UpdateTabIndices(const uint32_t startIdx);
         Windows::Foundation::Collections::IVector<TerminalApp::Command> _allTabActions{ nullptr };
         uint32_t _switcherStartIdx;
         void _anchorKeyUpHandler();

src/cascadia/TerminalApp/Tab.cpp

@@ -8,6 +8,8 @@
 #include "Tab.g.cpp"
 #include "Utils.h"
 #include "ColorHelper.h"
+#include "ActionAndArgs.h"
+#include "ActionArgs.h"
 
 using namespace winrt;
 using namespace winrt::Windows::UI::Xaml;
@@ -35,6 +37,7 @@ namespace winrt::TerminalApp::implementation
         Content(_rootPane->GetRootElement());
 
         _MakeTabViewItem();
+        _MakeSwitchToTabCommand();
     }
 
     // Method Description:
@@ -212,6 +215,9 @@ namespace winrt::TerminalApp::implementation
             // The TabViewItem Icon needs MUX while the IconSourceElement in the CommandPalette needs WUX...
             IconSource(GetColoredIcon<winrt::WUX::Controls::IconSource>(_lastIconPath));
             _tabViewItem.IconSource(GetColoredIcon<winrt::MUX::Controls::IconSource>(_lastIconPath));
+
+            // Update SwitchToTab command's icon
+            SwitchToTabCommand().IconSource(IconSource());
         }
     }
 
@@ -249,6 +255,9 @@ namespace winrt::TerminalApp::implementation
             // Bubble our current tab text to anyone who's listening for changes.
             Title(GetActiveTitle());
 
+            // Update SwitchToTab command's name
+            SwitchToTabCommand().Name(Title());
+
             // Update the UI to reflect the changed
             _UpdateTabHeader();
         }
@@ -1027,6 +1036,35 @@ namespace winrt::TerminalApp::implementation
         return _zoomedPane != nullptr;
     }
 
+    // Method Description:
+    // - Initializes a SwitchToTab command object for this Tab instance.
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - <none>
+    void Tab::_MakeSwitchToTabCommand()
+    {
+        auto focusTabAction = winrt::make_self<implementation::ActionAndArgs>();
+        auto args = winrt::make_self<implementation::SwitchToTabArgs>();
+        args->TabIndex(_TabViewIndex);
+
+        focusTabAction->Action(ShortcutAction::SwitchToTab);
+        focusTabAction->Args(*args);
+
+        winrt::TerminalApp::Command command;
+        command.Action(*focusTabAction);
+        command.Name(Title());
+        command.IconSource(IconSource());
+
+        SwitchToTabCommand(command);
+    }
+
+    void Tab::UpdateTabViewIndex(const uint32_t idx)
+    {
+        TabViewIndex(idx);
+        SwitchToTabCommand().Action().Args().as<implementation::SwitchToTabArgs>()->TabIndex(idx);
+    }
+
     DEFINE_EVENT(Tab, ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>);
     DEFINE_EVENT(Tab, ColorSelected, _colorSelected, winrt::delegate<winrt::Windows::UI::Color>);
     DEFINE_EVENT(Tab, ColorCleared, _colorCleared, winrt::delegate<>);

src/cascadia/TerminalApp/Tab.h

@@ -67,6 +67,8 @@ namespace winrt::TerminalApp::implementation
 
         int GetLeafPaneCount() const noexcept;
 
+        void UpdateTabViewIndex(const uint32_t idx);
+
         WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
         DECLARE_EVENT(ActivePaneChanged, _ActivePaneChangedHandlers, winrt::delegate<>);
@@ -75,6 +77,11 @@ namespace winrt::TerminalApp::implementation
 
         OBSERVABLE_GETSET_PROPERTY(winrt::hstring, Title, _PropertyChangedHandlers);
         OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::Controls::IconSource, IconSource, _PropertyChangedHandlers, nullptr);
+        OBSERVABLE_GETSET_PROPERTY(winrt::TerminalApp::Command, SwitchToTabCommand, _PropertyChangedHandlers, nullptr);
+
+        // The TabViewIndex is the index this Tab object resides in TerminalPage's _tabs vector.
+        // This is needed since Tab is going to be managing its own SwitchToTab command.
+        OBSERVABLE_GETSET_PROPERTY(uint32_t, TabViewIndex, _PropertyChangedHandlers, 0);
 
         OBSERVABLE_GETSET_PROPERTY(winrt::Windows::UI::Xaml::UIElement, Content, _PropertyChangedHandlers, nullptr);
 
@@ -115,6 +122,8 @@ namespace winrt::TerminalApp::implementation
         void _ApplyTabColor(const winrt::Windows::UI::Color& color);
         void _ClearTabBackgroundColor();
 
+        void _MakeSwitchToTabCommand();
+
         friend class ::TerminalAppLocalTests::TabTests;
     };
 }

src/cascadia/TerminalApp/Tab.idl

@@ -1,12 +1,16 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT license.
 
+import "Command.idl";
+
 namespace TerminalApp
 {
     [default_interface] runtimeclass Tab : Windows.UI.Xaml.Data.INotifyPropertyChanged
     {
         String Title { get; };
         Windows.UI.Xaml.Controls.IconSource IconSource { get; };
         Windows.UI.Xaml.UIElement Content { get; };
+        Command SwitchToTabCommand { get; };
+        UInt32 TabViewIndex { get; };
     }
 }

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -165,6 +165,7 @@ namespace winrt::TerminalApp::implementation
                     auto tab = tabs.GetAt(from.value());
                     tabs.RemoveAt(from.value());
                     tabs.InsertAt(to.value(), tab);
+                    page->_UpdateTabIndices();
                 }
 
                 page->_rearranging = false;
@@ -701,6 +702,9 @@ namespace winrt::TerminalApp::implementation
         auto newTabImpl = winrt::make_self<Tab>(profileGuid, term);
         _tabs.Append(*newTabImpl);
 
+        // Give the tab its index in the _tabs vector so it can manage its own SwitchToTab command.
+        newTabImpl->UpdateTabViewIndex(_tabs.Size() - 1);
+
         // Hookup our event handlers to the new terminal
         _RegisterTerminalEvents(term, *newTabImpl);
 
@@ -1074,6 +1078,7 @@ namespace winrt::TerminalApp::implementation
 
         _tabs.RemoveAt(tabIndex);
         _tabView.TabItems().RemoveAt(tabIndex);
+        _UpdateTabIndices();
 
         // To close the window here, we need to close the hosting window.
         if (_tabs.Size() == 0)
@@ -2547,6 +2552,20 @@ namespace winrt::TerminalApp::implementation
         return _isAlwaysOnTop;
     }
 
+    // Method Description:
+    // - Updates all tabs with their current index in _tabs.
+    // Arguments:
+    // - <none>
+    // Return Value:
+    // - <none>
+    void TerminalPage::_UpdateTabIndices()
+    {
+        for (uint32_t i = 0; i < _tabs.Size(); ++i)
+        {
+            _GetStrongTabImpl(i)->UpdateTabViewIndex(i);
+        }
+    }
+
     // -------------------------------- WinRT Events ---------------------------------
     // Winrt events need a method for adding a callback to the event and removing the callback.
     // These macros will define them both for you.

src/cascadia/TerminalApp/TerminalPage.h

@@ -93,6 +93,7 @@ namespace winrt::TerminalApp::implementation
         Windows::Foundation::Collections::IObservableVector<TerminalApp::Tab> _tabs;
         winrt::com_ptr<Tab> _GetStrongTabImpl(const uint32_t index) const;
         winrt::com_ptr<Tab> _GetStrongTabImpl(const ::winrt::TerminalApp::Tab& tab) const;
+        void _UpdateTabIndices();
 
         bool _isInFocusMode{ false };
         bool _isFullscreen{ false };