Fix wt --help (#9246)

When the user executes `--help`, make sure we force the creation of a new window, so that the `MessageBox` will actually appear. Add tests too. * [x] I work here * [x] Fixes #9230 * [x] Tests added I'm gonna have to immediately rewrite those tests for https://github.com/microsoft/terminal/projects/5#card-51431478, but this issue is ship-blocking so I don't care
microsoft · Feb 22, 2021 · 99ffaa8 · 99ffaa8
1 parent c9dea60
commit 99ffaa8
Show file tree

Hide file tree

Showing 4 changed files with 143 additions and 2 deletions.
diff --git a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
@@ -6,16 +6,24 @@
 
 #include "../types/inc/utils.hpp"
 #include "../TerminalApp/TerminalPage.h"
+#include "../TerminalApp/AppLogic.h"
 #include "../TerminalApp/AppCommandlineArgs.h"
+#include "../inc/WindowingBehavior.h"
 
 using namespace WEX::Logging;
 using namespace WEX::Common;
 using namespace WEX::TestExecution;
 
+using namespace winrt;
 using namespace winrt::Microsoft::Terminal::Settings::Model;
 using namespace winrt::TerminalApp;
 using namespace ::TerminalApp;
 
+namespace winrt
+{
+    namespace appImpl = TerminalApp::implementation;
+}
+
 namespace TerminalAppLocalTests
 {
     // TODO:microsoft/terminal#3838:
@@ -64,6 +72,10 @@ namespace TerminalAppLocalTests
 
         TEST_METHOD(TestMultipleSplitPaneSizes);
 
+        TEST_METHOD(TestFindTargetWindow);
+        TEST_METHOD(TestFindTargetWindowHelp);
+        TEST_METHOD(TestFindTargetWindowVersion);
+
     private:
         void _buildCommandlinesHelper(AppCommandlineArgs& appArgs,
                                       const size_t expectedSubcommands,
@@ -620,7 +632,7 @@ namespace TerminalAppLocalTests
         {
             AppCommandlineArgs appArgs{};
             std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"--tabColor", L"#009999" };
-            const auto expectedColor = Microsoft::Console::Utils::ColorFromHexString("#009999");
+            const auto expectedColor = ::Microsoft::Console::Utils::ColorFromHexString("#009999");
 
             _buildCommandlinesHelper(appArgs, 1u, rawCommands);
 
@@ -1578,4 +1590,106 @@ namespace TerminalAppLocalTests
             }
         }
     }
+
+    void CommandlineTest::TestFindTargetWindow()
+    {
+        {
+            std::vector<winrt::hstring> args{ L"wt.exe" };
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseAnyExisting,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        }
+        {
+            std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"-1" };
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        }
+        {
+            std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"-12345" };
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        }
+        {
+            std::vector<winrt::hstring> args{ L"wt.exe", L"-w", L"0" };
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseCurrent,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        }
+        {
+            std::vector<winrt::hstring> args{ L"wt.exe", L"new-tab" };
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseExisting,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseAnyExisting,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        }
+    }
+
+    void CommandlineTest::TestFindTargetWindowHelp()
+    {
+        Log::Comment(L"--help should always create a new window");
+
+        // This is a little helper to make sure that these args _always_ return
+        // UseNew, regardless of the windowing behavior.
+        auto testHelper = [](auto&& args) {
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        };
+
+        testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"--help" });
+        testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"new-tab", L"--help" });
+        testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"-w", L"0", L"new-tab", L"--help" });
+        testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"new-tab", L";", L"--help" });
+    }
+
+    void CommandlineTest::TestFindTargetWindowVersion()
+    {
+        Log::Comment(L"--version should always create a new window");
+
+        // This is a little helper to make sure that these args _always_ return
+        // UseNew, regardless of the windowing behavior.
+        auto testHelper = [](auto&& args) {
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseNew));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseExisting));
+
+            VERIFY_ARE_EQUAL(WindowingBehaviorUseNew,
+                             appImpl::AppLogic::_doFindTargetWindow({ args }, WindowingMode::UseAnyExisting));
+        };
+
+        testHelper(std::vector<winrt::hstring>{ L"wt.exe", L"--version" });
+    }
 }
diff --git a/src/cascadia/LocalTests_TerminalApp/pch.h b/src/cascadia/LocalTests_TerminalApp/pch.h
@@ -62,6 +62,8 @@ Author(s):
 #include <regex>
 #include <CLI11/CLI11.hpp>
 
+#include <shobjidl_core.h>
+
 // Manually include til after we include Windows.Foundation to give it winrt superpowers
 #include "til.h"
 

diff --git a/src/cascadia/TerminalApp/AppLogic.cpp b/src/cascadia/TerminalApp/AppLogic.cpp
@@ -1215,11 +1215,23 @@ namespace winrt::TerminalApp::implementation
     //   window ON ANY DESKTOP"
     // - anything else: We should handle the commandline in the window with the given ID.
     int32_t AppLogic::FindTargetWindow(array_view<const winrt::hstring> args)
+    {
+        return AppLogic::_doFindTargetWindow(args, _settings.GlobalSettings().WindowingBehavior());
+    }
+
+    // The main body of this function is a static helper, to facilitate unit-testing
+    int32_t AppLogic::_doFindTargetWindow(array_view<const winrt::hstring> args,
+                                          const Microsoft::Terminal::Settings::Model::WindowingMode& windowingBehavior)
     {
         ::TerminalApp::AppCommandlineArgs appArgs;
         const auto result = appArgs.ParseArgs(args);
         if (result == 0)
         {
+            if (!appArgs.GetExitMessage().empty())
+            {
+                return WindowingBehaviorUseNew;
+            }
+
             const auto parsedTarget = appArgs.GetTargetWindow();
             if (parsedTarget.has_value())
             {
@@ -1233,7 +1245,6 @@ namespace winrt::TerminalApp::implementation
                 // If the user did not provide any value on the commandline,
                 // then lookup our windowing behavior to determine what to do
                 // now.
-                const auto windowingBehavior = _settings.GlobalSettings().WindowingBehavior();
                 switch (windowingBehavior)
                 {
                 case WindowingMode::UseExisting:

diff --git a/src/cascadia/TerminalApp/AppLogic.h b/src/cascadia/TerminalApp/AppLogic.h
@@ -8,6 +8,14 @@
 #include "Jumplist.h"
 #include "../../cascadia/inc/cppwinrt_utils.h"
 
+#ifdef UNIT_TESTING
+// fwdecl unittest classes
+namespace TerminalAppLocalTests
+{
+    class CommandlineTest;
+};
+#endif
+
 namespace winrt::TerminalApp::implementation
 {
     struct AppLogic : AppLogicT<AppLogic, IInitializeWithWindow>
@@ -90,6 +98,8 @@ namespace winrt::TerminalApp::implementation
         ::TerminalApp::AppCommandlineArgs _appArgs;
         ::TerminalApp::AppCommandlineArgs _settingsAppArgs;
         int _ParseArgs(winrt::array_view<const hstring>& args);
+        static int32_t _doFindTargetWindow(winrt::array_view<const hstring> args,
+                                           const Microsoft::Terminal::Settings::Model::WindowingMode& windowingBehavior);
 
         void _ShowLoadErrorsDialog(const winrt::hstring& titleKey, const winrt::hstring& contentKey, HRESULT settingsLoadedResult);
         void _ShowLoadWarningsDialog();
@@ -125,6 +135,10 @@ namespace winrt::TerminalApp::implementation
         FORWARDED_TYPED_EVENT(AlwaysOnTopChanged, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, AlwaysOnTopChanged);
         FORWARDED_TYPED_EVENT(RaiseVisualBell, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, RaiseVisualBell);
         FORWARDED_TYPED_EVENT(SetTaskbarProgress, winrt::Windows::Foundation::IInspectable, winrt::Windows::Foundation::IInspectable, _root, SetTaskbarProgress);
+
+#ifdef UNIT_TESTING
+        friend class TerminalAppLocalTests::CommandlineTest;
+#endif
     };
 }