Display ATS tabs in MRU order (#7952)

This PR changes the ATS display order to _always_ be in most recently
used (MRU) order. I chose not to give ATS the option to be displayed
in-order because that order is better served through the traditional
left-right TabRow switching. 

_Note_: `TabSearch` will stay in-order.

This means that users can only choose one order or another in their
`nextTab/prevTab` bindings. Setting `useTabSwitcher` to true will make
nT/pT open the ATS in MRU order. If it's set to false, the ATS won't
open and nT/pT will simply go left and right on the TabRow.

I'm open to getting rid of the global and making ATS its own keybinding,
but for now I figured I would keep the current behavior and open the PR
to get eyes on the code that doesn't have anything to do with the
settings.

Closes #973

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -489,6 +489,14 @@ namespace winrt::TerminalApp::implementation
     void TerminalPage::_HandleOpenTabSearch(const IInspectable& /*sender*/,
                                             const ActionEventArgs& args)
     {
+        // Tab search is always in-order.
+        auto tabCommands = winrt::single_threaded_vector<Command>();
+        for (const auto& tab : _tabs)
+        {
+            tabCommands.Append(tab.SwitchToTabCommand());
+        }
+        CommandPalette().SetTabActions(tabCommands);
+
         auto opt = _GetFocusedTabIndex();
         uint32_t startIdx = opt.value_or(0);
 

src/cascadia/TerminalApp/CommandPalette.cpp

@@ -615,6 +615,12 @@ namespace winrt::TerminalApp::implementation
         _updateFilteredActions();
     }
 
+    void CommandPalette::SetTabActions(Collections::IVector<Command> const& tabs)
+    {
+        _allTabActions = tabs;
+        _updateFilteredActions();
+    }
+
     void CommandPalette::EnableCommandPaletteMode()
     {
         _switchToMode(CommandPaletteMode::ActionMode);
@@ -976,45 +982,6 @@ namespace winrt::TerminalApp::implementation
         _currentNestedCommands.Clear();
     }
 
-    // Method Description:
-    // - Listens for changes to TerminalPage's _tabs vector. Updates our vector of
-    //   tab switching commands accordingly.
-    // Arguments:
-    // - s: The vector being listened to.
-    // - e: The vector changed args that tells us whether a change, insert, or removal was performed
-    //      on the listened-to vector.
-    // Return Value:
-    // - <none>
-    void CommandPalette::OnTabsChanged(const IInspectable& s, const IVectorChangedEventArgs& e)
-    {
-        if (auto tabList = s.try_as<IObservableVector<TerminalApp::Tab>>())
-        {
-            auto idx = e.Index();
-            auto changedEvent = e.CollectionChange();
-
-            switch (changedEvent)
-            {
-            case CollectionChange::ItemChanged:
-            {
-                break;
-            }
-            case CollectionChange::ItemInserted:
-            {
-                auto tab = tabList.GetAt(idx);
-                _allTabActions.InsertAt(idx, tab.SwitchToTabCommand());
-                break;
-            }
-            case CollectionChange::ItemRemoved:
-            {
-                _allTabActions.RemoveAt(idx);
-                break;
-            }
-            }
-
-            _updateFilteredActions();
-        }
-    }
-
     void CommandPalette::EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx)
     {
         _switcherStartIdx = startIdx;

src/cascadia/TerminalApp/CommandPalette.h

@@ -23,6 +23,7 @@ namespace winrt::TerminalApp::implementation
         Windows::Foundation::Collections::IObservableVector<Microsoft::Terminal::Settings::Model::Command> FilteredActions();
 
         void SetCommands(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& actions);
+        void SetTabActions(Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command> const& tabs);
         void SetKeyBindings(Microsoft::Terminal::TerminalControl::IKeyBindings bindings);
 
         void EnableCommandPaletteMode();
@@ -35,7 +36,6 @@ namespace winrt::TerminalApp::implementation
 
         // Tab Switcher
         void EnableTabSwitcherMode(const bool searchMode, const uint32_t startIdx);
-        void OnTabsChanged(const Windows::Foundation::IInspectable& s, const Windows::Foundation::Collections::IVectorChangedEventArgs& e);
 
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
         OBSERVABLE_GETSET_PROPERTY(winrt::hstring, NoMatchesText, _PropertyChangedHandlers);

src/cascadia/TerminalApp/CommandPalette.idl

@@ -19,6 +19,7 @@ namespace TerminalApp
         Windows.Foundation.Collections.IObservableVector<Microsoft.Terminal.Settings.Model.Command> FilteredActions { get; };
 
         void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
+        void SetTabActions(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> tabs);
         void SetKeyBindings(Microsoft.Terminal.TerminalControl.IKeyBindings bindings);
         void EnableCommandPaletteMode();
 
@@ -27,6 +28,5 @@ namespace TerminalApp
         void SetDispatch(ShortcutActionDispatch dispatch);
 
         void EnableTabSwitcherMode(Boolean searchMode, UInt32 startIdx);
-        void OnTabsChanged(IInspectable s, Windows.Foundation.Collections.IVectorChangedEventArgs e);
     }
 }

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -43,6 +43,7 @@ namespace winrt::TerminalApp::implementation
 {
     TerminalPage::TerminalPage() :
         _tabs{ winrt::single_threaded_observable_vector<TerminalApp::Tab>() },
+        _mruTabActions{ winrt::single_threaded_vector<Command>() },
         _startupActions{ winrt::single_threaded_vector<ActionAndArgs>() }
     {
         InitializeComponent();
@@ -223,13 +224,6 @@ namespace winrt::TerminalApp::implementation
             }
         });
 
-        _tabs.VectorChanged([weakThis{ get_weak() }](auto&& s, auto&& e) {
-            if (auto page{ weakThis.get() })
-            {
-                page->CommandPalette().OnTabsChanged(s, e);
-            }
-        });
-
         // Settings AllowDependentAnimations will affect whether animations are
         // enabled application-wide, so we don't need to check it each time we
         // want to create an animation.
@@ -671,6 +665,7 @@ namespace winrt::TerminalApp::implementation
         // Add the new tab to the list of our tabs.
         auto newTabImpl = winrt::make_self<Tab>(profileGuid, term);
         _tabs.Append(*newTabImpl);
+        _mruTabActions.Append(newTabImpl->SwitchToTabCommand());
 
         newTabImpl->SetDispatch(*_actionDispatch);
 
@@ -1048,6 +1043,13 @@ namespace winrt::TerminalApp::implementation
         auto tab{ _GetStrongTabImpl(tabIndex) };
         tab->Shutdown();
 
+        uint32_t mruIndex;
+        if (_mruTabActions.IndexOf(_tabs.GetAt(tabIndex).SwitchToTabCommand(), mruIndex))
+        {
+            _mruTabActions.RemoveAt(mruIndex);
+            CommandPalette().SetTabActions(_mruTabActions);
+        }
+
         _tabs.RemoveAt(tabIndex);
         _tabView.TabItems().RemoveAt(tabIndex);
         _UpdateTabIndices();
@@ -1176,29 +1178,34 @@ namespace winrt::TerminalApp::implementation
     // - Sets focus to the tab to the right or left the currently selected tab.
     void TerminalPage::_SelectNextTab(const bool bMoveRight)
     {
-        if (auto index{ _GetFocusedTabIndex() })
+        if (_settings.GlobalSettings().UseTabSwitcher())
         {
-            uint32_t tabCount = _tabs.Size();
-            // Wraparound math. By adding tabCount and then calculating modulo tabCount,
-            // we clamp the values to the range [0, tabCount) while still supporting moving
-            // leftward from 0 to tabCount - 1.
-            const auto newTabIndex = ((tabCount + *index + (bMoveRight ? 1 : -1)) % tabCount);
+            CommandPalette().SetTabActions(_mruTabActions);
 
-            if (_settings.GlobalSettings().UseTabSwitcher())
-            {
-                if (CommandPalette().Visibility() == Visibility::Visible)
-                {
-                    CommandPalette().SelectNextItem(bMoveRight);
-                }
+            // Since ATS is always MRU, our focused tab index is always 0.
+            // So, going next should go to index 1, and going prev should wrap to the end.
+            uint32_t tabCount = _mruTabActions.Size();
+            auto newTabIndex = ((tabCount + (bMoveRight ? 1 : -1)) % tabCount);
 
-                CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
-                CommandPalette().Visibility(Visibility::Visible);
+            if (CommandPalette().Visibility() == Visibility::Visible)
+            {
+                CommandPalette().SelectNextItem(bMoveRight);
             }
             else
             {
-                _SelectTab(newTabIndex);
+                CommandPalette().EnableTabSwitcherMode(false, newTabIndex);
+                CommandPalette().Visibility(Visibility::Visible);
             }
         }
+        else if (auto index{ _GetFocusedTabIndex() })
+        {
+            uint32_t tabCount = _tabs.Size();
+            // Wraparound math. By adding tabCount and then calculating modulo tabCount,
+            // we clamp the values to the range [0, tabCount) while still supporting moving
+            // leftward from 0 to tabCount - 1.
+            auto newTabIndex = ((tabCount + *index + (bMoveRight ? 1 : -1)) % tabCount);
+            _SelectTab(newTabIndex);
+        }
     }
 
     // Method Description:
@@ -1960,6 +1967,7 @@ namespace winrt::TerminalApp::implementation
                 if (CommandPalette().Visibility() != Visibility::Visible)
                 {
                     tab->SetFocused(true);
+                    _UpdateMRUTab(index);
                 }
 
                 // Raise an event that our title changed
@@ -2495,6 +2503,7 @@ namespace winrt::TerminalApp::implementation
         if (auto index{ _GetFocusedTabIndex() })
         {
             _GetStrongTabImpl(index.value())->SetFocused(true);
+            _UpdateMRUTab(index.value());
         }
     }
 
@@ -2536,6 +2545,27 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
+    // Method Description:
+    // - Bumps the tab in its in-order index up to the top of the mru list.
+    // Arguments:
+    // - index: the in-order index of the tab to bump.
+    // Return Value:
+    // - <none>
+    void TerminalPage::_UpdateMRUTab(const uint32_t index)
+    {
+        uint32_t mruIndex;
+        auto command = _tabs.GetAt(index).SwitchToTabCommand();
+        if (_mruTabActions.IndexOf(command, mruIndex))
+        {
+            if (mruIndex > 0)
+            {
+                _mruTabActions.RemoveAt(mruIndex);
+                _mruTabActions.InsertAt(0, command);
+                CommandPalette().SetTabActions(_mruTabActions);
+            }
+        }
+    }
+
     // -------------------------------- 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

@@ -89,6 +89,7 @@ namespace winrt::TerminalApp::implementation
         Microsoft::Terminal::Settings::Model::CascadiaSettings _settings{ nullptr };
 
         Windows::Foundation::Collections::IObservableVector<TerminalApp::Tab> _tabs;
+        Windows::Foundation::Collections::IVector<winrt::Microsoft::Terminal::Settings::Model::Command> _mruTabActions;
         winrt::com_ptr<Tab> _GetStrongTabImpl(const uint32_t index) const;
         winrt::com_ptr<Tab> _GetStrongTabImpl(const ::winrt::TerminalApp::Tab& tab) const;
         void _UpdateTabIndices();
@@ -205,6 +206,9 @@ namespace winrt::TerminalApp::implementation
 
         void _UnZoomIfNeeded();
 
+        void _UpdateTabSwitcherCommands(const bool mru);
+        void _UpdateMRUTab(const uint32_t index);
+
 #pragma region ActionHandlers
         // These are all defined in AppActionHandlers.cpp
         void _HandleOpenNewTabDropdown(const IInspectable& sender, const Microsoft::Terminal::Settings::Model::ActionEventArgs& args);