From c5208e615878c57285d11990734c8f2afc569bd9 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Thu, 9 Jul 2020 12:37:02 -0500
Subject: [PATCH] finish remaining todos, also, good bot

---
 .../LocalTests_TerminalApp/SettingsTests.cpp  | 37 +++++++++++++++++--
 src/cascadia/TerminalApp/Command.cpp          |  4 +-
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
index 9a90948dfa..3f3ac16623 100644
--- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
@@ -83,7 +83,7 @@ namespace TerminalAppLocalTests
         TEST_METHOD(TestCommandsAndKeybindings);
 
         TEST_METHOD(TestIterateCommands);
-        TEST_METHOD(TestIterateAutogenNamedCommands);
+        TEST_METHOD(TestIterateOnGeneratedNamedCommands);
         TEST_METHOD(TestIterateOnBadJson);
         TEST_METHOD(TestNestedCommands);
         TEST_METHOD(TestNestedInNestedCommand);
@@ -2698,7 +2698,7 @@ namespace TerminalAppLocalTests
         }
     }
 
-    void SettingsTests::TestIterateAutogenNamedCommands()
+    void SettingsTests::TestIterateOnGeneratedNamedCommands()
     {
         // For this test, put an iterable command without a given `name` to
         // replace. When we expand it, it should still work.
@@ -3699,7 +3699,7 @@ namespace TerminalAppLocalTests
 
     void SettingsTests::TestUnbindNestedCommand()
     {
-        // TODO: description
+        // Test that layering a command with `"commands": null` set will unbind a command that already exists.
 
         const std::string settingsJson{ R"(
         {
@@ -3781,7 +3781,8 @@ namespace TerminalAppLocalTests
 
     void SettingsTests::TestRebindNestedCommand()
     {
-        // TODO: description
+        // Test that layering a command with an action set on top of a command
+        // with nested commands replaces the nested commands with an action.
 
         const std::string settingsJson{ R"(
         {
@@ -3851,6 +3852,17 @@ namespace TerminalAppLocalTests
         VERIFY_ARE_EQUAL(0u, settings._warnings.size());
         VERIFY_ARE_EQUAL(1u, commands.size());
 
+        {
+            winrt::hstring commandName{ L"parent" };
+            auto commandProj = commands.at(commandName);
+            VERIFY_IS_NOT_NULL(commandProj);
+
+            winrt::com_ptr<implementation::Command> commandImpl;
+            commandImpl.copy_from(winrt::get_self<implementation::Command>(commandProj));
+
+            VERIFY_ARE_EQUAL(2u, commandImpl->_subcommands.size());
+        }
+
         Log::Comment(L"Layer second bit of json, to unbind the original command.");
         settings._ParseJsonString(settings1Json, false);
         settings.LayerJson(settings._userSettings);
@@ -3858,6 +3870,23 @@ namespace TerminalAppLocalTests
         _logCommandNames(commands);
         VERIFY_ARE_EQUAL(0u, settings._warnings.size());
         VERIFY_ARE_EQUAL(1u, commands.size());
+
+        {
+            winrt::hstring commandName{ L"parent" };
+            auto commandProj = commands.at(commandName);
+
+            VERIFY_IS_NOT_NULL(commandProj);
+            auto actionAndArgs = commandProj.Action();
+            VERIFY_IS_NOT_NULL(actionAndArgs);
+            VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action());
+            const auto& realArgs = actionAndArgs.Args().try_as<NewTabArgs>();
+            VERIFY_IS_NOT_NULL(realArgs);
+
+            winrt::com_ptr<implementation::Command> commandImpl;
+            commandImpl.copy_from(winrt::get_self<implementation::Command>(commandProj));
+
+            VERIFY_ARE_EQUAL(0u, commandImpl->_subcommands.size());
+        }
     }
 
 }
diff --git a/src/cascadia/TerminalApp/Command.cpp b/src/cascadia/TerminalApp/Command.cpp
index a2962ca5d6..21f588d3d1 100644
--- a/src/cascadia/TerminalApp/Command.cpp
+++ b/src/cascadia/TerminalApp/Command.cpp
@@ -255,7 +255,7 @@ namespace winrt::TerminalApp::implementation
 
     // Function Description:
     // - Helper to escape a string as a json string. This function will also
-    //   trim off the leading and trailing double-qoutes, so the output string
+    //   trim off the leading and trailing double-quotes, so the output string
     //   can be inserted directly into another json blob.
     // Arguments:
     // - input: the string to JSON escape.
@@ -298,7 +298,7 @@ namespace winrt::TerminalApp::implementation
     }
 
     // Method Description:
-    // - Iterate over all the provided comamnds, and recursively expand any
+    // - Iterate over all the provided commands, and recursively expand any
     //   commands with `iterateOn` set. If we successfully generated expanded
     //   commands for them, then we'll remove the original command, and add all
     //   the newly generated commands.