From 817df81f4893f5d618148ff3ec3b16ffc088a483 Mon Sep 17 00:00:00 2001 From: Don-Vito Date: Wed, 18 Nov 2020 23:04:35 +0200 Subject: [PATCH] Fix combining wt args and "wt new-tab" args in implicit context (#8315) Currently when implicit tab command is specified (i.e., we have parameters for new-tab, but don't have the explicit subcommand name) we fallback to parsing the entire arg list as new tab command. However, if we also have a launch profile (or anything else that might in the future belong to the upper scope) it is passed as a parameter to the new tab command, failing the parsing. The idea behind my solution is to run the parser as a prefix command - i.e., as long as we succeed to parse [options] / [subcommand] we will parse them (populating the fields like a launch mode), but once we will discover something unfamiliar, like profile, we will know that the prefix is over and will handle the remaining suffix as a new tab command. ## Validation Steps Performed * UT added * Manual run of different options Closes #7318 (cherry picked from commit 435e45726e9366e7aef64cde9bdbe325bfe25d72) --- .../CommandlineTest.cpp | 53 ++++++++++++++++++ .../TerminalApp/AppCommandlineArgs.cpp | 54 ++++++++----------- 2 files changed, 74 insertions(+), 33 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp index 0c9fe6df64e..57c4b4f0730 100644 --- a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp +++ b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp @@ -58,6 +58,7 @@ namespace TerminalAppLocalTests TEST_METHOD(TestMultipleCommandExecuteCommandlineAction); TEST_METHOD(TestInvalidExecuteCommandlineAction); TEST_METHOD(TestLaunchMode); + TEST_METHOD(TestLaunchModeWithNoCommand); private: void _buildCommandlinesHelper(AppCommandlineArgs& appArgs, @@ -1224,4 +1225,56 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(appArgs.GetLaunchMode().value(), LaunchMode::MaximizedFocusMode); } } + + void CommandlineTest::TestLaunchModeWithNoCommand() + { + { + Log::Comment(NoThrowString().Format(L"Pass a launch mode and profile")); + + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", L"-M", L"--profile", L"cmd" }; + _buildCommandlinesHelper(appArgs, 1u, rawCommands); + + VERIFY_IS_TRUE(appArgs.GetLaunchMode().has_value()); + VERIFY_ARE_EQUAL(appArgs.GetLaunchMode().value(), LaunchMode::MaximizedMode); + VERIFY_ARE_EQUAL(1u, appArgs._startupActions.size()); + + auto actionAndArgs = appArgs._startupActions.at(0); + VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + auto myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_IS_NOT_NULL(myArgs.TerminalArgs()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().Commandline().empty()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().StartingDirectory().empty()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().TabTitle().empty()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().ProfileIndex() == nullptr); + VERIFY_IS_FALSE(myArgs.TerminalArgs().Profile().empty()); + VERIFY_ARE_EQUAL(L"cmd", myArgs.TerminalArgs().Profile()); + } + { + Log::Comment(NoThrowString().Format(L"Pass a launch mode and command line")); + + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", L"-M", L"powershell.exe" }; + _buildCommandlinesHelper(appArgs, 1u, rawCommands); + + VERIFY_IS_TRUE(appArgs.GetLaunchMode().has_value()); + VERIFY_ARE_EQUAL(appArgs.GetLaunchMode().value(), LaunchMode::MaximizedMode); + VERIFY_ARE_EQUAL(1u, appArgs._startupActions.size()); + + auto actionAndArgs = appArgs._startupActions.at(0); + VERIFY_ARE_EQUAL(ShortcutAction::NewTab, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + auto myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_IS_NOT_NULL(myArgs.TerminalArgs()); + VERIFY_IS_FALSE(myArgs.TerminalArgs().Commandline().empty()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().StartingDirectory().empty()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().TabTitle().empty()); + VERIFY_IS_TRUE(myArgs.TerminalArgs().ProfileIndex() == nullptr); + VERIFY_IS_TRUE(myArgs.TerminalArgs().Profile().empty()); + VERIFY_ARE_EQUAL(L"powershell.exe", myArgs.TerminalArgs().Commandline()); + } + } } diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp index 413956a0aa0..684ee12cf7e 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp @@ -69,50 +69,29 @@ int AppCommandlineArgs::ParseCommand(const Commandline& command) { throw CLI::CallForHelp(); } - // Clear the parser's internal state - _app.clear(); - // attempt to parse the commandline + // attempt to parse the commandline prefix of the form [options][subcommand] _app.parse(args); + auto remainingParams = _app.remaining_size(); // If we parsed the commandline, and _no_ subcommands were provided, try - // parsing again as a "new-tab" command. - + // parse the remaining suffix as a "new-tab" command. if (_noCommandsProvided()) { - _newTabCommand.subcommand->clear(); _newTabCommand.subcommand->parse(args); + remainingParams = _newTabCommand.subcommand->remaining_size(); + } + + // if after parsing the prefix and (optionally) the implicit tab subcommand + // we still have unparsed parameters we need to fail + if (remainingParams > 0) + { + throw CLI::ExtrasError(args); } - } - catch (const CLI::CallForHelp& e) - { - return _handleExit(_app, e); } catch (const CLI::ParseError& e) { - // If we parsed the commandline, and _no_ subcommands were provided, try - // parsing again as a "new-tab" command. - if (_noCommandsProvided()) - { - try - { - // CLI11 mutated the original vector the first time it tried to - // parse the args. Reconstruct it the way CLI11 wants here. - // "See above for why it's begin() + 1" - std::vector args{ command.Args().begin() + 1, command.Args().end() }; - std::reverse(args.begin(), args.end()); - _newTabCommand.subcommand->clear(); - _newTabCommand.subcommand->parse(args); - } - catch (const CLI::ParseError& e) - { - return _handleExit(*_newTabCommand.subcommand, e); - } - } - else - { - return _handleExit(_app, e); - } + return _handleExit(_app, e); } return 0; } @@ -157,6 +136,15 @@ int AppCommandlineArgs::_handleExit(const CLI::App& command, const CLI::Error& e // - void AppCommandlineArgs::_buildParser() { + // We define or parser as a prefix command, to support "implicit new tab subcommand" scenario. + // In this scenario we will try to parse the prefix that contains parameters like launch mode, + // but will not encounter an explicit command. + // Instead we will encounter an argument that doesn't belong to the prefix indicating the prefix is over. + // Then we will try to parse the remaining arguments as a new tab subcommand. + // E.g., for "wt.exe -M -d c:/", we will use -M for the launch mode, but once we will encounter -d + // we will know that the prefix is over and try to handle the suffix as a new tab subcommand + _app.prefix_command(); + // -v,--version: Displays version info auto versionCallback = [this](int64_t /*count*/) { // Set our message to display the application name and the current version.