From 99ffaa8a1af7e9a92e84ec5a680c2bf62d77baa7 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 22 Feb 2021 12:50:39 -0600 Subject: [PATCH] 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 --- .../CommandlineTest.cpp | 116 +++++++++++++++++- src/cascadia/LocalTests_TerminalApp/pch.h | 2 + src/cascadia/TerminalApp/AppLogic.cpp | 13 +- src/cascadia/TerminalApp/AppLogic.h | 14 +++ 4 files changed, 143 insertions(+), 2 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp index c28c2e0b04b..4cab5a32d9d 100644 --- 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 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 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 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 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 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 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{ L"wt.exe", L"--help" }); + testHelper(std::vector{ L"wt.exe", L"new-tab", L"--help" }); + testHelper(std::vector{ L"wt.exe", L"-w", L"0", L"new-tab", L"--help" }); + testHelper(std::vector{ 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{ L"wt.exe", L"--version" }); + } } diff --git a/src/cascadia/LocalTests_TerminalApp/pch.h b/src/cascadia/LocalTests_TerminalApp/pch.h index 95217495ab6..737bdf1792e 100644 --- a/src/cascadia/LocalTests_TerminalApp/pch.h +++ b/src/cascadia/LocalTests_TerminalApp/pch.h @@ -62,6 +62,8 @@ Author(s): #include #include +#include + // 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 index 3597f229355..4f2fb7bdeb8 100644 --- 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 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 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 index 50613501c51..d011738c977 100644 --- 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 @@ -90,6 +98,8 @@ namespace winrt::TerminalApp::implementation ::TerminalApp::AppCommandlineArgs _appArgs; ::TerminalApp::AppCommandlineArgs _settingsAppArgs; int _ParseArgs(winrt::array_view& args); + static int32_t _doFindTargetWindow(winrt::array_view 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 }; }