From a5eba5ac9ca4e80b254d51e3e723b999e41c9917 Mon Sep 17 00:00:00 2001
From: Mike Griese <migrie@microsoft.com>
Date: Tue, 7 Jul 2020 16:46:31 -0500
Subject: [PATCH] add some stubs for tests in the future

---
 .../LocalTests_TerminalApp/CommandTests.cpp   | 22 +++++++++++++++
 src/cascadia/TerminalApp/Command.cpp          | 28 ++++++++-----------
 2 files changed, 34 insertions(+), 16 deletions(-)

diff --git a/src/cascadia/LocalTests_TerminalApp/CommandTests.cpp b/src/cascadia/LocalTests_TerminalApp/CommandTests.cpp
index 129e246918..57a1e5eb09 100644
--- a/src/cascadia/LocalTests_TerminalApp/CommandTests.cpp
+++ b/src/cascadia/LocalTests_TerminalApp/CommandTests.cpp
@@ -41,6 +41,10 @@ namespace TerminalAppLocalTests
         TEST_METHOD(TestAutogeneratedName);
         TEST_METHOD(TestLayerOnAutogeneratedName);
 
+        TEST_METHOD(TestIterateCommands);
+        TEST_METHOD(TestIterateAutogenNamedCommands);
+        TEST_METHOD(TestIterateOnBadJson);
+
         TEST_CLASS_SETUP(ClassSetup)
         {
             InitializeJsonReader();
@@ -346,4 +350,22 @@ namespace TerminalAppLocalTests
             VERIFY_ARE_EQUAL(winrt::TerminalApp::SplitState::Vertical, realArgs.SplitStyle());
         }
     }
+    void CommandTests::TestIterateCommands()
+    {
+        // For this test, put an iterable command with a given `name`,
+        // containing a ${profile.name} to replace. When we expand it, it should
+        // have created one command for each profile.
+    }
+
+    void CommandTests::TestIterateAutogenNamedCommands()
+    {
+        // For this test, put an iterable command without a given `name` to
+        // replace. When we expand it, it should still work.
+    }
+    void CommandTests::TestIterateOnBadJson()
+    {
+        // For this test, put an iterable command with a profile name that would
+        // cause bad json to be filled in. Something like a profile with a name
+        // of "Foo\"", so the trailing '"' might break the json parsing.
+    }
 }
diff --git a/src/cascadia/TerminalApp/Command.cpp b/src/cascadia/TerminalApp/Command.cpp
index 22e9c43ba9..c3043fd17d 100644
--- a/src/cascadia/TerminalApp/Command.cpp
+++ b/src/cascadia/TerminalApp/Command.cpp
@@ -112,20 +112,15 @@ namespace winrt::TerminalApp::implementation
     {
         auto result = winrt::make_self<Command>();
 
-        // bool parseActionLater = false;
-
         if (const auto iterateOnJson{ json[JsonKey(IterateOnKey)] })
         {
             auto s = iterateOnJson.asString();
             if (s == IterateOnProfilesValue)
             {
                 result->_IterateOn = ExpandCommandType::Profiles;
-                // parseActionLater = true;
             }
         }
 
-        // if (!parseActionLater)
-        // {
         // TODO GH#6644: iconPath not implemented quite yet. Can't seem to get
         // the binding quite right. Additionally, do we want it to be an image,
         // or a FontIcon? I've had difficulty binding either/or.
@@ -146,6 +141,11 @@ namespace winrt::TerminalApp::implementation
                 return nullptr;
             }
 
+            // If an iterable command doesn't have a name set, we'll still just
+            // try and generate a fake name for the command give the string we
+            // currently have. It'll probably generate something like "New tab,
+            // profile: ${profile.name}". This string will only be temporarily
+            // used internally, so there's no problem.
             result->_setName(_nameFromJsonOrAction(json, actionAndArgs));
         }
         else
@@ -154,17 +154,12 @@ namespace winrt::TerminalApp::implementation
             // should also be used for unbinding.
             return nullptr;
         }
-        // }
-        // else
-        // {
-        // // Just use the current string as the name for now.
-        // result->_setName(_nameFromJson(json));
-        result->_originalJson = json;
-        // }
 
-        // TODO: an iterable command might not have a name set at all, and might
-        // be relying on the command to be expanded, then have the name auto
-        // generated. Make sure that those types of commands will still work.
+        // Stash the original json value in this object. If the command is
+        // iterable, we'll need to re-parse it later, once we know what all the
+        // values we can iterate on are.
+        result->_originalJson = json;
+
         if (result->_Name.empty())
         {
             return nullptr;
@@ -289,7 +284,6 @@ namespace winrt::TerminalApp::implementation
                 //   then re-attempt to parse the action and args.
                 //
                 auto newCmd = winrt::make_self<Command>();
-                // newCmd->_setIconPath(expandable->_IconPath);
                 newCmd->_setKeyChordText(expandable->_KeyChordText);
                 newCmd->_setName(_replaceKeyword(expandable->_Name,
                                                  ProfileName,
@@ -313,6 +307,8 @@ namespace winrt::TerminalApp::implementation
                     break;
                 }
 
+                // TODO: We should probably just pass this back though FromJson
+
                 // auto actionAndArgs = _getActionAndArgsFromJson(newJsonValue, warnings);
 
                 if (const auto actionJson{ newJsonValue[JsonKey(ActionKey)] })