misc

src/cascadia/TerminalSettingsModel/ActionMap.h

@@ -111,10 +111,21 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         bool _fixUpsAppliedDuringLoad;
 
         void _AddKeyBindingHelper(const Json::Value& json, std::vector<SettingsLoadWarnings>& warnings);
+
+        // _KeyMap is the map of key chords -> action IDs defined in this layer
+        // _ActionMap is the map of action IDs -> commands defined in this layer
+        // These maps are the ones that we deserialize into when parsing the user json and vice-versa
         std::unordered_map<Control::KeyChord, winrt::hstring, KeyChordHash, KeyChordEquality> _KeyMap;
         std::unordered_map<winrt::hstring, Model::Command> _ActionMap;
+
+        // _CumulativeKeyMapCache is the map of key chords -> action IDs defined in all layers, with child layers overriding parent layers
         Windows::Foundation::Collections::IMap<Control::KeyChord, winrt::hstring> _CumulativeKeyMapCache{ nullptr };
+        // _CumulativeActionMapCache is the map of action IDs -> commands defined in all layers, with child layers overriding parent layers
         Windows::Foundation::Collections::IMap<winrt::hstring, Model::Command> _CumulativeActionMapCache{ nullptr };
+
+        // _ResolvedKeyActionMapCache is the map of key chords -> commands defined in all layers, with child layers overriding parent layers
+        // This is effectively a combination of _CumulativeKeyMapCache and _CumulativeActionMapCache and its purpose is so that
+        // we can give the SUI a view of the key chords and the commands they map to
         Windows::Foundation::Collections::IMap<Control::KeyChord, Model::Command> _ResolvedKeyActionMapCache{ nullptr };
 
         friend class SettingsModelUnitTests::KeyBindingsTests;

src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp

@@ -65,21 +65,27 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             {
                 AddAction(*Command::FromJson(jsonBlock, warnings, origin, withKeybindings));
 
-                // this is a 'command' block and there are keys - meaning this is the legacy style
-                // let the loader know that fixups are needed
-                if (jsonBlock.isMember(JsonKey("keys")))
+                // 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("command")) && !jsonBlock.isMember(JsonKey("iterateOn")))
                 {
-                    _fixUpsAppliedDuringLoad = true;
-                }
-                // for non-nested non-iterable user commands, if there's no ID we generate one for them
-                // let the loader know that fixups are needed
-                if (origin == OriginTag::User && !jsonBlock.isMember(JsonKey("id")) && jsonBlock.isMember(JsonKey("command")) && !jsonBlock.isMember(JsonKey("iterateOn")))
-                {
-                    _fixUpsAppliedDuringLoad = true;
+                    if (jsonBlock.isMember(JsonKey("keys")))
+                    {
+                        // there are keys in this command block - its the legacy style
+                        _fixUpsAppliedDuringLoad = true;
+                    }
+
+                    if (origin == OriginTag::User && !jsonBlock.isMember(JsonKey("id")))
+                    {
+                        // there's no ID in this command block - we will generate one for the user
+                        // inform the loader that the ID needs to be written into the json
+                        _fixUpsAppliedDuringLoad = true;
+                    }
                 }
             }
             else
             {
+                // this is not a command block, so it is a keybinding block
                 _AddKeyBindingHelper(jsonBlock, warnings);
             }
         }