Give Command ownership over KeyChord

src/cascadia/TerminalApp/ActionPaletteItem.cpp

@@ -22,7 +22,7 @@ namespace winrt::TerminalApp::implementation
         _Command(command)
     {
         Name(command.Name());
-        KeyChordText(command.KeyChordText());
+        KeyChordText(KeyChordSerialization::ToString(command.Keys()));
         Icon(command.IconPath());
 
         _commandChangedRevoker = command.PropertyChanged(winrt::auto_revoke, [weakThis{ get_weak() }](auto& sender, auto& e) {
@@ -38,7 +38,7 @@ namespace winrt::TerminalApp::implementation
                 }
                 else if (changedProperty == L"KeyChordText")
                 {
-                    item->KeyChordText(senderCommand.KeyChordText());
+                    item->KeyChordText(KeyChordSerialization::ToString(senderCommand.Keys()));
                 }
                 else if (changedProperty == L"IconPath")
                 {

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -74,17 +74,17 @@ namespace winrt::TerminalApp::implementation
         for (const auto& nameAndCmd : commands)
         {
             const auto& command = nameAndCmd.Value();
+
             // If there's a keybinding that's bound to exactly this command,
-            // then get the string for that keychord and display it as a
-            // part of the command in the UI. Each Command's KeyChordText is
+            // then get the keychord and display it as a
+            // part of the command in the UI. Each Command's KeyChord is
             // unset by default, so we don't need to worry about clearing it
             // if there isn't a key associated with it.
-            auto keyChord{ settings.KeyMap().GetKeyBindingForActionWithArgs(command.Action()) };
+            // However, if a nested command tries to bind a KeyChord, the
+            // Command records it, but it is never actually bound to the keys.
+            const auto keyChord{ settings.KeyMap().GetKeyBindingForActionWithArgs(command.Action()) };
+            command.Keys(keyChord);
 
-            if (keyChord)
-            {
-                command.KeyChordText(KeyChordSerialization::ToString(keyChord));
-            }
             if (command.HasNestedCommands())
             {
                 _recursiveUpdateCommandKeybindingLabels(settings, command.NestedCommands());

src/cascadia/TerminalSettingsEditor/Actions.cpp

@@ -31,7 +31,7 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             // Filter out nested commands, and commands that aren't bound to a
             // key. This page is currently just for displaying the actions that
             // _are_ bound to keys.
-            if (command.HasNestedCommands() || command.KeyChordText().empty())
+            if (command.HasNestedCommands() || !command.Keys())
             {
                 continue;
             }

src/cascadia/TerminalSettingsEditor/Actions.xaml

@@ -19,6 +19,8 @@
             </ResourceDictionary.MergedDictionaries>
 
             <local:StringIsEmptyConverter x:Key="CommandKeyChordVisibilityConverter" />
+            <local:KeyChordToStringConverter x:Key="KeyChordToStringConverter" />
+            <local:KeyChordToVisibilityConverter x:Key="KeyChordToVisibilityConverter" />
 
             <!--
                 Template for actions. This is _heavily_ copied from the command
@@ -43,7 +45,7 @@
                     to make sure it takes the entire width of the line
                 -->
                 <ListViewItem HorizontalContentAlignment="Stretch"
-                              AutomationProperties.AcceleratorKey="{x:Bind KeyChordText, Mode=OneWay}"
+                              AutomationProperties.AcceleratorKey="{x:Bind Keys, Mode=OneWay, Converter={StaticResource KeyChordToStringConverter}}"
                               AutomationProperties.Name="{x:Bind Name, Mode=OneWay}">
 
                     <Grid HorizontalAlignment="Stretch"
@@ -76,11 +78,11 @@
                                 HorizontalAlignment="Right"
                                 VerticalAlignment="Center"
                                 Style="{ThemeResource KeyChordBorderStyle}"
-                                Visibility="{x:Bind KeyChordText, Mode=OneWay, Converter={StaticResource CommandKeyChordVisibilityConverter}}">
+                                Visibility="{x:Bind Keys, Mode=OneWay, Converter={StaticResource KeyChordToVisibilityConverter}}">
 
                             <TextBlock FontSize="12"
                                        Style="{ThemeResource KeyChordTextBlockStyle}"
-                                       Text="{x:Bind KeyChordText, Mode=OneWay}" />
+                                       Text="{x:Bind Keys, Mode=OneWay, Converter={StaticResource KeyChordToStringConverter}}" />
                         </Border>
                     </Grid>
                 </ListViewItem>

src/cascadia/TerminalSettingsEditor/Converters.idl

@@ -59,4 +59,14 @@ namespace Microsoft.Terminal.Settings.Editor
         DesktopWallpaperToEmptyStringConverter();
     };
 
+    runtimeclass KeyChordToVisibilityConverter : [default] Windows.UI.Xaml.Data.IValueConverter
+    {
+        KeyChordToVisibilityConverter();
+    };
+
+    runtimeclass KeyChordToStringConverter : [default] Windows.UI.Xaml.Data.IValueConverter
+    {
+        KeyChordToStringConverter();
+    };
+
 }

src/cascadia/TerminalSettingsEditor/KeyChordConverter.cpp

@@ -0,0 +1,48 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#include "pch.h"
+#include "KeyChordConverter.h"
+#include "KeyChordToStringConverter.g.cpp"
+#include "KeyChordToVisibilityConverter.g.cpp"
+
+using namespace winrt::Windows;
+using namespace winrt::Windows::UI::Xaml;
+using namespace winrt::Windows::UI::Text;
+
+namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
+{
+    Foundation::IInspectable KeyChordToStringConverter::Convert(Foundation::IInspectable const& value,
+                                                                Windows::UI::Xaml::Interop::TypeName const& /* targetType */,
+                                                                Foundation::IInspectable const& /* parameter */,
+                                                                hstring const& /* language */)
+    {
+        const auto& keys{ winrt::unbox_value<Control::KeyChord>(value) };
+        return box_value(Model::KeyChordSerialization::ToString(keys));
+    }
+
+    Foundation::IInspectable KeyChordToStringConverter::ConvertBack(Foundation::IInspectable const& value,
+                                                                    Windows::UI::Xaml::Interop::TypeName const& /* targetType */,
+                                                                    Foundation::IInspectable const& /*parameter*/,
+                                                                    hstring const& /* language */)
+    {
+        const auto& keys{ winrt::unbox_value<hstring>(value) };
+        return box_value(Model::KeyChordSerialization::FromString(keys));
+    }
+
+    Foundation::IInspectable KeyChordToVisibilityConverter::Convert(Foundation::IInspectable const& value,
+                                                                    Windows::UI::Xaml::Interop::TypeName const& /* targetType */,
+                                                                    Foundation::IInspectable const& /* parameter */,
+                                                                    hstring const& /* language */)
+    {
+        return box_value(value ? Visibility::Visible : Visibility::Collapsed);
+    }
+
+    Foundation::IInspectable KeyChordToVisibilityConverter::ConvertBack(Foundation::IInspectable const& /*value*/,
+                                                                        Windows::UI::Xaml::Interop::TypeName const& /* targetType */,
+                                                                        Foundation::IInspectable const& /*parameter*/,
+                                                                        hstring const& /* language */)
+    {
+        throw hresult_not_implemented();
+    }
+}

src/cascadia/TerminalSettingsEditor/KeyChordConverter.h

@@ -0,0 +1,11 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT license.
+
+#pragma once
+
+#include "KeyChordToStringConverter.g.h"
+#include "KeyChordToVisibilityConverter.g.h"
+#include "../inc/cppwinrt_utils.h"
+
+DECLARE_CONVERTER(winrt::Microsoft::Terminal::Settings::Editor, KeyChordToStringConverter);
+DECLARE_CONVERTER(winrt::Microsoft::Terminal::Settings::Editor, KeyChordToVisibilityConverter);

src/cascadia/TerminalSettingsEditor/Microsoft.Terminal.Settings.Editor.vcxproj

@@ -56,6 +56,9 @@
     <ClInclude Include="InvertedBooleanToVisibilityConverter.h">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClInclude>
+    <ClInclude Include="KeyChordConverter.h">
+      <DependentUpon>Converters.idl</DependentUpon>
+    </ClInclude>
     <ClInclude Include="StringIsEmptyConverter.h">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClInclude>
@@ -156,6 +159,9 @@
     <ClCompile Include="InvertedBooleanToVisibilityConverter.cpp">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClCompile>
+    <ClCompile Include="KeyChordConverter.cpp">
+      <DependentUpon>Converters.idl</DependentUpon>
+    </ClCompile>
     <ClCompile Include="StringIsEmptyConverter.cpp">
       <DependentUpon>Converters.idl</DependentUpon>
     </ClCompile>

src/cascadia/TerminalSettingsModel/Command.cpp

@@ -6,7 +6,7 @@
 #include "Command.g.cpp"
 
 #include "ActionAndArgs.h"
-#include "JsonUtils.h"
+#include "KeyChordSerialization.h"
 #include <LibraryResources.h>
 #include "TerminalSettingsSerializationHelpers.h"
 
@@ -26,6 +26,7 @@ static constexpr std::string_view ActionKey{ "command" };
 static constexpr std::string_view ArgsKey{ "args" };
 static constexpr std::string_view IterateOnKey{ "iterateOn" };
 static constexpr std::string_view CommandsKey{ "commands" };
+static constexpr std::string_view KeysKey{ "keys" };
 
 static constexpr std::string_view ProfileNameToken{ "${profile.name}" };
 static constexpr std::string_view ProfileIconToken{ "${profile.icon}" };
@@ -43,7 +44,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         auto command{ winrt::make_self<Command>() };
         command->_Name = _Name;
         command->_Action = _Action;
-        command->_KeyChordText = _KeyChordText;
+        command->_Keys = _Keys;
         command->_IconPath = _IconPath;
         command->_IterateOn = _IterateOn;
 
@@ -178,6 +179,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         }
 
         JsonUtils::GetValueForKey(json, IconKey, result->_IconPath);
+        JsonUtils::GetValueForKey(json, KeysKey, result->_Keys);
 
         // If we're a nested command, we can ignore the current action.
         if (!nested)

src/cascadia/TerminalSettingsModel/Command.h

@@ -56,7 +56,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         WINRT_CALLBACK(PropertyChanged, Windows::UI::Xaml::Data::PropertyChangedEventHandler);
         WINRT_OBSERVABLE_PROPERTY(winrt::hstring, Name, _PropertyChangedHandlers);
         WINRT_OBSERVABLE_PROPERTY(Model::ActionAndArgs, Action, _PropertyChangedHandlers);
-        WINRT_OBSERVABLE_PROPERTY(winrt::hstring, KeyChordText, _PropertyChangedHandlers);
+        WINRT_OBSERVABLE_PROPERTY(Control::KeyChord, Keys, _PropertyChangedHandlers, nullptr);
         WINRT_OBSERVABLE_PROPERTY(winrt::hstring, IconPath, _PropertyChangedHandlers);
 
         WINRT_PROPERTY(ExpandCommandType, IterateOn, ExpandCommandType::None);

src/cascadia/TerminalSettingsModel/Command.idl

@@ -14,7 +14,7 @@ namespace Microsoft.Terminal.Settings.Model
 
         String Name;
         ActionAndArgs Action;
-        String KeyChordText;
+        Microsoft.Terminal.Control.KeyChord Keys;
 
         String IconPath;
 

src/cascadia/TerminalSettingsModel/KeyChordSerialization.cpp

@@ -7,6 +7,7 @@
 
 using namespace winrt::Microsoft::Terminal::Control;
 using namespace winrt::Microsoft::Terminal::Settings::Model::implementation;
+using namespace Microsoft::Terminal::Settings::Model::JsonUtils;
 
 static constexpr std::wstring_view CTRL_KEY{ L"ctrl" };
 static constexpr std::wstring_view SHIFT_KEY{ L"shift" };
@@ -103,10 +104,8 @@ static const std::unordered_map<std::wstring_view, int32_t> vkeyNamePairs {
 // - hstr: the string to parse into a keychord.
 // Return Value:
 // - a newly constructed KeyChord
-KeyChord KeyChordSerialization::FromString(const winrt::hstring& hstr)
+static KeyChord _FromString(const std::wstring& wstr)
 {
-    std::wstring wstr{ hstr };
-
     // Split the string on '+'
     std::wstring temp;
     std::vector<std::wstring> parts;
@@ -215,8 +214,13 @@ KeyChord KeyChordSerialization::FromString(const winrt::hstring& hstr)
 //   names listed in the vkeyNamePairs vector above, or is one of 0-9a-z.
 // Return Value:
 // - a string which is an equivalent serialization of this object.
-winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord)
+static std::wstring _ToString(const KeyChord& chord)
 {
+    if (!chord)
+    {
+        return {};
+    }
+
     bool serializedSuccessfully = false;
     const auto modifiers = chord.Modifiers();
     const auto vkey = chord.Vkey();
@@ -282,5 +286,45 @@ winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord)
         buffer = L"";
     }
 
-    return winrt::hstring{ buffer };
+    return buffer;
+}
+
+KeyChord KeyChordSerialization::FromString(const winrt::hstring& hstr)
+{
+    return _FromString(std::wstring{ hstr });
+}
+
+winrt::hstring KeyChordSerialization::ToString(const KeyChord& chord)
+{
+    return hstring{ _ToString(chord) };
+}
+
+KeyChord ConversionTrait<KeyChord>::FromJson(const Json::Value& json)
+{
+    try
+    {
+        const std::string keyChordText{ json.isString() ?
+                                            json.asString() :
+                                            json[0].asString() };
+        return _FromString(til::u8u16(keyChordText));
+    }
+    catch (winrt::hresult_invalid_argument)
+    {
+        return nullptr;
+    }
+}
+
+bool ConversionTrait<KeyChord>::CanConvert(const Json::Value& json)
+{
+    return json.isString() || (json.isArray() && json.size() == 1);
+}
+
+Json::Value ConversionTrait<KeyChord>::ToJson(const KeyChord& val)
+{
+    return til::u16u8(_ToString(val));
+}
+
+std::string ConversionTrait<KeyChord>::TypeDescription() const
+{
+    return "key chord";
 }

src/cascadia/TerminalSettingsModel/KeyChordSerialization.h

@@ -4,6 +4,7 @@
 #pragma once
 
 #include "KeyChordSerialization.g.h"
+#include "JsonUtils.h"
 #include "../inc/cppwinrt_utils.h"
 
 namespace winrt::Microsoft::Terminal::Settings::Model::implementation
@@ -17,6 +18,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     };
 }
 
+template<>
+struct Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<winrt::Microsoft::Terminal::Control::KeyChord>
+{
+    winrt::Microsoft::Terminal::Control::KeyChord FromJson(const Json::Value& json);
+    bool CanConvert(const Json::Value& json);
+    Json::Value ToJson(const winrt::Microsoft::Terminal::Control::KeyChord& val);
+    std::string TypeDescription() const;
+};
+
 namespace winrt::Microsoft::Terminal::Settings::Model::factory_implementation
 {
     // C++/WinRT generates a constructor even though one is not specified in the IDL