update add action

Author: PankajBhojwani

src/cascadia/TerminalSettingsModel/ActionMap.cpp

@@ -449,7 +449,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     // - Adds a command to the ActionMap
     // Arguments:
     // - cmd: the command we're adding
-    void ActionMap::AddAction(const Model::Command& cmd)
+    void ActionMap::AddAction(const Model::Command& cmd, const Control::KeyChord& keys)
     {
         // _Never_ add null to the ActionMap
         if (!cmd)
@@ -489,7 +489,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         //  Add the new keybinding to the _KeyMap
 
         _TryUpdateActionMap(cmd);
-        _TryUpdateKeyChord(cmd);
+        _TryUpdateKeyChord(cmd, keys);
     }
 
     // Method Description:
@@ -548,12 +548,11 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     // - Update our internal state with the key chord of the newly registered action
     // Arguments:
     // - cmd: the action we're trying to register
-    void ActionMap::_TryUpdateKeyChord(const Model::Command& cmd)
+    void ActionMap::_TryUpdateKeyChord(const Model::Command& cmd, const Control::KeyChord& keys)
     {
         // Example (this is a legacy case, where the keys are provided in the same block as the command):
         //   {                "command": "copy", "keys": "ctrl+c" } --> we are registering a new key chord
         //   { "name": "foo", "command": "copy" }                   --> no change to keys, exit early
-        const auto keys{ cmd.Keys() };
         if (!keys)
         {
             // the user is not trying to update the keys.
@@ -773,7 +772,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         cmd->RegisterKey(keys);
         cmd->ActionAndArgs(action);
         cmd->GenerateID();
-        AddAction(*cmd);
+        AddAction(*cmd, keys);
     }
 
     // This is a helper to aid in sorting commands by their `Name`s, alphabetically.

src/cascadia/TerminalSettingsModel/ActionMap.h

@@ -64,7 +64,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         Control::KeyChord GetKeyBindingForAction(winrt::hstring cmdID) const;
 
         // population
-        void AddAction(const Model::Command& cmd);
+        void AddAction(const Model::Command& cmd, const Control::KeyChord& keys);
 
         // JSON
         static com_ptr<ActionMap> FromJson(const Json::Value& json, const OriginTag origin = OriginTag::None);
@@ -98,7 +98,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         void _PopulateCumulativeActionMap(std::unordered_map<hstring, Model::Command>& actionMap);
 
         void _TryUpdateActionMap(const Model::Command& cmd);
-        void _TryUpdateKeyChord(const Model::Command& cmd);
+        void _TryUpdateKeyChord(const Model::Command& cmd, const Control::KeyChord& keys);
 
         Windows::Foundation::Collections::IMap<hstring, Model::ActionAndArgs> _AvailableActionsCache{ nullptr };
         Windows::Foundation::Collections::IMap<hstring, Model::Command> _NameMapCache{ nullptr };

src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp

@@ -61,21 +61,32 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             // if there is no "command" field, then it is a modern style keys block
             if (jsonBlock.isMember(JsonKey(CommandsKey)) || jsonBlock.isMember(JsonKey(ActionKey)))
             {
-                AddAction(*Command::FromJson(jsonBlock, warnings, origin, withKeybindings));
-
-                // for non-nested non-iterable commands,
-                // check if this is a legacy-style command block so we can inform the loader that fixups are needed
-                if (jsonBlock.isMember(JsonKey(ActionKey)) && !jsonBlock.isMember(JsonKey(IterateOnKey)))
+                Control::KeyChord keys{ nullptr };
+                if (jsonBlock.isMember(JsonKey(KeysKey)))
                 {
-                    if (jsonBlock.isMember(JsonKey(KeysKey)))
+                    const auto keysJson{ json[JsonKey(KeysKey)] };
+                    if (keysJson.isArray() && keysJson.size() > 1)
                     {
-                        // there are keys in this command block - its the legacy style
+                        warnings.push_back(SettingsLoadWarnings::TooManyKeysForChord);
+                    }
+                    else
+                    {
+                        JsonUtils::GetValueForKey(json, KeysKey, keys);
+
+                        // there are keys in this command block meaning this is the legacy style -
+                        // inform the loader that fixups are needed
                         _fixUpsAppliedDuringLoad = true;
                     }
+                }
+
+                AddAction(*Command::FromJson(jsonBlock, warnings, origin, withKeybindings), keys);
 
+                if (jsonBlock.isMember(JsonKey(ActionKey)) && !jsonBlock.isMember(JsonKey(IterateOnKey)))
+                {
                     if (origin == OriginTag::User && !jsonBlock.isMember(JsonKey(IDKey)))
                     {
-                        // there's no ID in this command block - we will generate one for the user
+                        // for non-nested non-iterable commands,
+                        // if there's no ID in the command block we will generate one for the user -
                         // inform the loader that the ID needs to be written into the json
                         _fixUpsAppliedDuringLoad = true;
                     }