only one tojson

microsoft · Apr 30, 2024 · ca4015f · ca4015f
1 parent 4c744e6
commit ca4015f
Show file tree

Hide file tree

Showing 3 changed files with 19 additions and 99 deletions.
diff --git a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp
@@ -27,15 +27,14 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     }
 
     // Method Description:
-    // - Deserialize an ActionMap from the array `json`. The json array should contain
-    //   an array of serialized `Command` objects.
-    // - These actions are added to the `ActionMap`, where we automatically handle
-    //   overwriting and unbinding actions.
+    // - Deserialize an ActionMap from the array `json`
+    // - The json array either contains an array of serialized `Command` objects,
+    //   or an array of keybindings
+    // - The actions are added to _ActionMap and the keybindings are added to _KeyMap
     // Arguments:
-    // - json: an array of Json::Value's to deserialize into our ActionMap.
+    // - json: an array of Json::Value's to deserialize into our _ActionMap and _KeyMap
     // Return value:
     // - a list of warnings encountered while deserializing the json
-    // todo: update this description
     std::vector<SettingsLoadWarnings> ActionMap::LayerJson(const Json::Value& json, const OriginTag origin, const bool withKeybindings)
     {
         // It's possible that the user provided keybindings have some warnings in
@@ -97,46 +96,27 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     {
         Json::Value actionList{ Json::ValueType::arrayValue };
 
-        // Command serializes to an array of JSON objects.
-        // This is because a Command may have multiple key chords associated with it.
-        // The name and icon are only serialized in the first object.
-        // Example:
-        // { "name": "Custom Copy", "command": "copy", "keys": "ctrl+c" }
-        // {                        "command": "copy", "keys": "ctrl+shift+c" }
-        // {                        "command": "copy", "keys": "ctrl+ins" }
-        auto toJsonSpecial = [&actionList](const Model::Command& cmd) {
+        auto toJson = [&actionList](const Model::Command& cmd) {
             const auto cmdImpl{ winrt::get_self<implementation::Command>(cmd) };
-            const auto& cmdJsonArray{ cmdImpl->ToJsonSpecial() };
-            for (const auto& cmdJson : cmdJsonArray)
-            {
-                actionList.append(cmdJson);
-            }
-        };
-
-        auto toJsonStandard = [&actionList](const Model::Command& cmd) {
-            const auto cmdImpl{ winrt::get_self<implementation::Command>(cmd) };
-            const auto& cmdJsonArray{ cmdImpl->ToJsonStandard() };
-            for (const auto& cmdJson : cmdJsonArray)
-            {
-                actionList.append(cmdJson);
-            }
+            const auto& cmdJson{ cmdImpl->ToJson() };
+            actionList.append(cmdJson);
         };
 
         for (const auto& [_, cmd] : _ActionMap)
         {
-            toJsonStandard(cmd);
+            toJson(cmd);
         }
 
         // Serialize all nested Command objects added in the current layer
         for (const auto& [_, cmd] : _NestedCommands)
         {
-            toJsonSpecial(cmd);
+            toJson(cmd);
         }
 
         // Serialize all iterable Command objects added in the current layer
         for (const auto& cmd : _IterableCommands)
         {
-            toJsonSpecial(cmd);
+            toJson(cmd);
         }
 
         return actionList;

diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp
@@ -428,27 +428,25 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     }
 
     // Function Description:
-    // - Serialize the Command into an array of json actions
+    // - Serialize the Command into a json value
     // Arguments:
     // - <none>
     // Return Value:
-    // - an array of serialized actions
-    Json::Value Command::ToJsonSpecial() const
+    // - a serialized command
+    Json::Value Command::ToJson() const
     {
-        Json::Value cmdList{ Json::ValueType::arrayValue };
+        Json::Value cmdJson{ Json::ValueType::objectValue };
 
         if (_nestedCommand || _IterateOn != ExpandCommandType::None)
         {
             // handle special commands
             // For these, we can trust _originalJson to be correct.
             // In fact, we _need_ to use it here because we don't actually deserialize `iterateOn`
             //   until we expand the command.
-            cmdList.append(_originalJson);
+            cmdJson = _originalJson;
         }
-        else if (_keyMappings.empty())
+        else
         {
-            // only write out one command
-            Json::Value cmdJson{ Json::ValueType::objectValue };
             JsonUtils::SetValueForKey(cmdJson, IconKey, _iconPath);
             JsonUtils::SetValueForKey(cmdJson, NameKey, _name);
             if (!_ID.empty())
@@ -460,66 +458,9 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             {
                 cmdJson[JsonKey(ActionKey)] = ActionAndArgs::ToJson(_ActionAndArgs);
             }
-
-            cmdList.append(cmdJson);
-        }
-        else
-        {
-            // we'll write out one command per key mapping
-            for (auto keys{ _keyMappings.begin() }; keys != _keyMappings.end(); ++keys)
-            {
-                Json::Value cmdJson{ Json::ValueType::objectValue };
-
-                if (keys == _keyMappings.begin())
-                {
-                    // First iteration also writes icon and name
-                    JsonUtils::SetValueForKey(cmdJson, IconKey, _iconPath);
-                    JsonUtils::SetValueForKey(cmdJson, NameKey, _name);
-                    if (!_ID.empty())
-                    {
-                        JsonUtils::SetValueForKey(cmdJson, IDKey, _ID);
-                    }
-                }
-
-                if (_ActionAndArgs)
-                {
-                    cmdJson[JsonKey(ActionKey)] = ActionAndArgs::ToJson(_ActionAndArgs);
-                }
-
-                JsonUtils::SetValueForKey(cmdJson, KeysKey, *keys);
-                cmdList.append(cmdJson);
-            }
         }
 
-        return cmdList;
-    }
-
-    // Function Description:
-    // - Serialize the Command into an array of json actions
-    // Arguments:
-    // - <none>
-    // Return Value:
-    // - an array of serialized actions
-    Json::Value Command::ToJsonStandard() const
-    {
-        Json::Value cmdList{ Json::ValueType::arrayValue };
-
-        Json::Value cmdJson{ Json::ValueType::objectValue };
-        JsonUtils::SetValueForKey(cmdJson, IconKey, _iconPath);
-        JsonUtils::SetValueForKey(cmdJson, NameKey, _name);
-        if (!_ID.empty())
-        {
-            JsonUtils::SetValueForKey(cmdJson, IDKey, _ID);
-        }
-
-        if (_ActionAndArgs)
-        {
-            cmdJson[JsonKey(ActionKey)] = ActionAndArgs::ToJson(_ActionAndArgs);
-        }
-
-        cmdList.append(cmdJson);
-
-        return cmdList;
+        return cmdJson;
     }
 
     // Function Description:

diff --git a/src/cascadia/TerminalSettingsModel/Command.h b/src/cascadia/TerminalSettingsModel/Command.h
@@ -50,8 +50,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         static std::vector<SettingsLoadWarnings> LayerJson(Windows::Foundation::Collections::IMap<winrt::hstring, Model::Command>& commands,
                                                            const Json::Value& json,
                                                            const OriginTag origin);
-        Json::Value ToJsonSpecial() const;
-        Json::Value ToJsonStandard() const;
+        Json::Value ToJson() const;
 
         bool HasNestedCommands() const;
         bool IsNestedCommand() const noexcept;