handle and test nested and unbound commands

microsoft · May 5, 2021 · 586b977 · 586b977
1 parent c434ee6
commit 586b977
Show file tree

Hide file tree

Showing 4 changed files with 60 additions and 8 deletions.
diff --git a/src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp b/src/cascadia/LocalTests_SettingsModel/SerializationTests.cpp
@@ -234,11 +234,9 @@ namespace SettingsModelLocalTests
         const std::string actionsString2A{ R"([
                                                 { "command": { "action": "setTabColor" } }
                                             ])" };
-
         const std::string actionsString2B{ R"([
                                                 { "command": { "action": "setTabColor", "color": "#112233" } }
                                             ])" };
-
         const std::string actionsString2C{ R"([
                                                 { "command": { "action": "copy" } },
                                                 { "command": { "action": "copy", "singleLine": true, "copyFormatting": "html" } }
@@ -262,10 +260,40 @@ namespace SettingsModelLocalTests
         const std::string actionsString6{ R"([
                                                 { "command": { "action": "newTab", "index": 0 }, "keys": "ctrl+g" },
                                             ])" };
+
         const std::string actionsString7{ R"([
                                                 { "command": { "action": "renameWindow", "name": null }, "keys": "ctrl+h" }
                                             ])" };
 
+        const std::string actionsString8{ R"([
+                                                {
+                                                    "name": "Change font size...",
+                                                    "commands": [
+                                                        { "command": { "action": "adjustFontSize", "delta": 1 } },
+                                                        { "command": { "action": "adjustFontSize", "delta": -1 } },
+                                                        { "command": "resetFontSize" },
+                                                    ]
+                                                }
+                                            ])" };
+
+        const std::string actionsString9{ R"([
+                                                {
+                                                    "name": "New tab",
+                                                    "commands": [
+                                                        {
+                                                            "iterateOn": "profiles",
+                                                            "icon": "${profile.icon}",
+                                                            "name": "${profile.name}",
+                                                            "command": { "action": "newTab", "profile": "${profile.name}" }
+                                                        }
+                                                    ]
+                                                }
+                                            ])" };
+
+        const std::string actionsString10{ R"([
+                                                { "command": "unbound", "keys": "ctrl+c" }
+                                            ])" };
+
         Log::Comment(L"simple command");
         RoundtripTest<implementation::ActionMap>(actionsString1);
 
@@ -288,6 +316,15 @@ namespace SettingsModelLocalTests
 
         Log::Comment(L"complex command with meaningful null arg");
         RoundtripTest<implementation::ActionMap>(actionsString7);
+
+        Log::Comment(L"nested command");
+        RoundtripTest<implementation::ActionMap>(actionsString8);
+
+        Log::Comment(L"iterable command");
+        RoundtripTest<implementation::ActionMap>(actionsString9);
+
+        Log::Comment(L"unbound command");
+        RoundtripTest<implementation::ActionMap>(actionsString10);
     }
 
     void SerializationTests::CascadiaSettings()

diff --git a/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionAndArgs.cpp
@@ -158,9 +158,7 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
         { ShortcutAction::NewWindow, NewWindowArgs::ToJson },
         { ShortcutAction::PrevTab, PrevTabArgs::ToJson },
         { ShortcutAction::NextTab, NextTabArgs::ToJson },
-        { ShortcutAction::RenameWindow, RenameWindowArgs::ToJson },
-
-        { ShortcutAction::Invalid, nullptr },
+        { ShortcutAction::RenameWindow, RenameWindowArgs::ToJson }
     };
 
     // Function Description:
@@ -301,8 +299,8 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
             }
         }
 
-        // "command": null
-        return { Json::ValueType::nullValue };
+        // "command": "unbound"
+        return JsonKey(UnboundKey);
     }
 
     com_ptr<ActionAndArgs> ActionAndArgs::Copy() const

diff --git a/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp b/src/cascadia/TerminalSettingsModel/ActionMapSerialization.cpp
@@ -68,6 +68,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
                 actionList.append(cmdJson);
             }
         }
+        for (const auto& [_, cmd] : _NestedCommands)
+        {
+            const auto cmdImpl{ winrt::get_self<implementation::Command>(cmd) };
+            const auto& cmdJsonArray{ cmdImpl->ToJson() };
+            for (const auto& cmdJson : cmdJsonArray)
+            {
+                actionList.append(cmdJson);
+            }
+        }
         return actionList;
     }
 

diff --git a/src/cascadia/TerminalSettingsModel/Command.cpp b/src/cascadia/TerminalSettingsModel/Command.cpp
@@ -393,7 +393,15 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation
     {
         Json::Value cmdList{ Json::ValueType::arrayValue };
 
-        if (_keyMappings.empty())
+        if (_nestedCommand)
+        {
+            // handle nested command
+            // For nested commands, 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);
+        }
+        else if (_keyMappings.empty())
         {
             // only write out one command
             Json::Value cmdJson{ Json::ValueType::objectValue };