Remove command's knowledge of its keys (#17215)

With the move to Action IDs, it doesn't quite make sense anymore for a `Command` to know which keys map to it. This PR removes all `Keys` from `Command`, and any callers to that now instead query the `ActionMap` for that Command's keys. Closes #17160 Closes #13943
microsoft · Jun 6, 2024 · d6b6aac · d6b6aac
1 parent 9317d42
commit d6b6aac
Show file tree

Hide file tree

Showing 20 changed files with 123 additions and 260 deletions.
diff --git a/.github/actions/spelling/allow/allow.txt b/.github/actions/spelling/allow/allow.txt
@@ -56,6 +56,7 @@ hyperlink
 hyperlinking
 hyperlinks
 iconify
+ID
 img
 inlined
 issuetitle

diff --git a/.github/actions/spelling/expect/expect.txt b/.github/actions/spelling/expect/expect.txt
@@ -822,6 +822,7 @@ idllib
 IDOK
 IDR
 idth
+IDTo
 IDXGI
 IEnd
 IEnum
@@ -1870,7 +1871,7 @@ unk
 unknwn
 UNORM
 unparseable
-unregistering
+Unregistering
 untextured
 UPDATEDISPLAY
 UPDOWN

diff --git a/src/cascadia/TerminalApp/ActionPaletteItem.cpp b/src/cascadia/TerminalApp/ActionPaletteItem.cpp
@@ -18,11 +18,11 @@ using namespace winrt::Microsoft::Terminal::Settings::Model;
 
 namespace winrt::TerminalApp::implementation
 {
-    ActionPaletteItem::ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command) :
+    ActionPaletteItem::ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command, const winrt::hstring keyChordText) :
         _Command(command)
     {
         Name(command.Name());
-        KeyChordText(command.KeyChordText());
+        KeyChordText(keyChordText);
         Icon(command.IconPath());
     }
 }
diff --git a/src/cascadia/TerminalApp/ActionPaletteItem.h b/src/cascadia/TerminalApp/ActionPaletteItem.h
@@ -11,7 +11,7 @@ namespace winrt::TerminalApp::implementation
     struct ActionPaletteItem : ActionPaletteItemT<ActionPaletteItem, PaletteItem>
     {
         ActionPaletteItem() = default;
-        ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command);
+        ActionPaletteItem(const Microsoft::Terminal::Settings::Model::Command& command, const winrt::hstring keyChordText);
 
         WINRT_PROPERTY(Microsoft::Terminal::Settings::Model::Command, Command, nullptr);
 

diff --git a/src/cascadia/TerminalApp/ActionPaletteItem.idl b/src/cascadia/TerminalApp/ActionPaletteItem.idl
@@ -7,7 +7,7 @@ namespace TerminalApp
 {
     [default_interface] runtimeclass ActionPaletteItem : PaletteItem
     {
-        ActionPaletteItem(Microsoft.Terminal.Settings.Model.Command command);
+        ActionPaletteItem(Microsoft.Terminal.Settings.Model.Command command, String keyChordText);
 
         Microsoft.Terminal.Settings.Model.Command Command { get; };
     }

diff --git a/src/cascadia/TerminalApp/CommandPalette.cpp b/src/cascadia/TerminalApp/CommandPalette.cpp
@@ -950,21 +950,27 @@ namespace winrt::TerminalApp::implementation
     void CommandPalette::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap)
     {
         _actionMap = actionMap;
+        _populateCommands();
     }
 
-    void CommandPalette::SetCommands(const Collections::IVector<Command>& actions)
+    void CommandPalette::_populateCommands()
     {
         _allCommands.Clear();
-        for (const auto& action : actions)
+        if (_actionMap)
         {
-            auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) };
-            auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) };
-            _allCommands.Append(filteredCommand);
-        }
+            const auto expandedCommands{ _actionMap.ExpandedCommands() };
+            for (const auto& action : expandedCommands)
+            {
+                const auto keyChordText{ KeyChordSerialization::ToString(_actionMap.GetKeyBindingForAction(action.ID())) };
+                auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, keyChordText) };
+                auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) };
+                _allCommands.Append(filteredCommand);
+            }
 
-        if (Visibility() == Visibility::Visible && _currentMode == CommandPaletteMode::ActionMode)
-        {
-            _updateFilteredActions();
+            if (Visibility() == Visibility::Visible && _currentMode == CommandPaletteMode::ActionMode)
+            {
+                _updateFilteredActions();
+            }
         }
     }
 
@@ -1178,7 +1184,8 @@ namespace winrt::TerminalApp::implementation
         for (const auto& nameAndCommand : parentCommand.NestedCommands())
         {
             const auto action = nameAndCommand.Value();
-            auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) };
+            // nested commands cannot have keys bound to them, so just pass in the command and no keys
+            auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, winrt::hstring{}) };
             auto nestedFilteredCommand{ winrt::make<FilteredCommand>(nestedActionPaletteItem) };
             _currentNestedCommands.Append(nestedFilteredCommand);
         }

diff --git a/src/cascadia/TerminalApp/CommandPalette.h b/src/cascadia/TerminalApp/CommandPalette.h
@@ -31,7 +31,6 @@ namespace winrt::TerminalApp::implementation
 
         Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::FilteredCommand> FilteredActions();
 
-        void SetCommands(const Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command>& actions);
         void SetTabs(const Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase>& tabs, const Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::TabBase>& mruTabs);
         void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap);
 
@@ -81,6 +80,8 @@ namespace winrt::TerminalApp::implementation
 
         bool _lastFilterTextWasEmpty{ true };
 
+        void _populateCommands();
+
         void _filterTextChanged(const Windows::Foundation::IInspectable& sender,
                                 const Windows::UI::Xaml::RoutedEventArgs& args);
         void _previewKeyDownHandler(const Windows::Foundation::IInspectable& sender,

diff --git a/src/cascadia/TerminalApp/CommandPalette.idl b/src/cascadia/TerminalApp/CommandPalette.idl
@@ -20,8 +20,6 @@ namespace TerminalApp
 
         Windows.Foundation.Collections.IObservableVector<FilteredCommand> FilteredActions { get; };
 
-        void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
-
         void SetTabs(Windows.Foundation.Collections.IObservableVector<TabBase> tabs, Windows.Foundation.Collections.IObservableVector<TabBase> mruTabs);
 
         void SetActionMap(Microsoft.Terminal.Settings.Model.IActionMapView actionMap);

diff --git a/src/cascadia/TerminalApp/SuggestionsControl.cpp b/src/cascadia/TerminalApp/SuggestionsControl.cpp
@@ -508,7 +508,7 @@ namespace winrt::TerminalApp::implementation
                         automationPeer.RaiseNotificationEvent(
                             Automation::Peers::AutomationNotificationKind::ItemAdded,
                             Automation::Peers::AutomationNotificationProcessing::MostRecent,
-                            paletteItem.Name() + L" " + paletteItem.KeyChordText(),
+                            paletteItem.Name(),
                             L"SuggestionsControlSelectedItemChanged" /* unique name for this notification category */);
                     }
                 }
@@ -751,17 +751,13 @@ namespace winrt::TerminalApp::implementation
         return _filteredActions;
     }
 
-    void SuggestionsControl::SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap)
-    {
-        _actionMap = actionMap;
-    }
-
     void SuggestionsControl::SetCommands(const Collections::IVector<Command>& actions)
     {
         _allCommands.Clear();
         for (const auto& action : actions)
         {
-            auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) };
+            // key chords aren't relevant in the suggestions control, so make the palette item with just the command and no keys
+            auto actionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, winrt::hstring{}) };
             auto filteredCommand{ winrt::make<FilteredCommand>(actionPaletteItem) };
             _allCommands.Append(filteredCommand);
         }
@@ -915,7 +911,7 @@ namespace winrt::TerminalApp::implementation
         for (const auto& nameAndCommand : parentCommand.NestedCommands())
         {
             const auto action = nameAndCommand.Value();
-            auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action) };
+            auto nestedActionPaletteItem{ winrt::make<winrt::TerminalApp::implementation::ActionPaletteItem>(action, winrt::hstring{}) };
             auto nestedFilteredCommand{ winrt::make<FilteredCommand>(nestedActionPaletteItem) };
             _currentNestedCommands.Append(nestedFilteredCommand);
         }

diff --git a/src/cascadia/TerminalApp/SuggestionsControl.h b/src/cascadia/TerminalApp/SuggestionsControl.h
@@ -24,7 +24,6 @@ namespace winrt::TerminalApp::implementation
         Windows::Foundation::Collections::IObservableVector<winrt::TerminalApp::FilteredCommand> FilteredActions();
 
         void SetCommands(const Windows::Foundation::Collections::IVector<Microsoft::Terminal::Settings::Model::Command>& actions);
-        void SetActionMap(const Microsoft::Terminal::Settings::Model::IActionMapView& actionMap);
 
         bool OnDirectKeyEvent(const uint32_t vkey, const uint8_t scanCode, const bool down);
 

diff --git a/src/cascadia/TerminalApp/SuggestionsControl.idl b/src/cascadia/TerminalApp/SuggestionsControl.idl
@@ -36,7 +36,6 @@ namespace TerminalApp
         SuggestionsMode Mode { get; set; };
 
         void SetCommands(Windows.Foundation.Collections.IVector<Microsoft.Terminal.Settings.Model.Command> actions);
-        void SetActionMap(Microsoft.Terminal.Settings.Model.IActionMapView actionMap);
         void SelectNextItem(Boolean moveDown);
 
         void Open(SuggestionsMode mode, IVector<Microsoft.Terminal.Settings.Model.Command> commands, String filterText, Windows.Foundation.Point anchor, Windows.Foundation.Size space, Single characterHeight);

diff --git a/src/cascadia/TerminalApp/SuggestionsControl.xaml b/src/cascadia/TerminalApp/SuggestionsControl.xaml
@@ -31,7 +31,6 @@
                               MinHeight="0"
                               Padding="16,0,12,0"
                               HorizontalContentAlignment="Stretch"
-                              AutomationProperties.AcceleratorKey="{x:Bind Item.KeyChordText, Mode=OneWay}"
                               AutomationProperties.Name="{x:Bind Item.Name, Mode=OneWay}"
                               FontSize="12" />
             </DataTemplate>

diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -123,7 +123,6 @@ namespace winrt::TerminalApp::implementation
         // to happen before the Settings UI is reloaded and tries to re-read those values.
         if (const auto p = CommandPaletteElement())
         {
-            p.SetCommands(_settings.GlobalSettings().ActionMap().ExpandedCommands());
             p.SetActionMap(_settings.ActionMap());
         }
 
@@ -1828,7 +1827,6 @@ namespace winrt::TerminalApp::implementation
     {
         const auto p = FindName(L"CommandPaletteElement").as<CommandPalette>();
 
-        p.SetCommands(_settings.GlobalSettings().ActionMap().ExpandedCommands());
         p.SetActionMap(_settings.ActionMap());
 
         // When the visibility of the command palette changes to "collapsed",