Skip to content

Commit

Permalink
Fix combining wt args and "wt new-tab" args in implicit context (#8315)
Browse files Browse the repository at this point in the history
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 435e457)
  • Loading branch information
Don-Vito authored and DHowett committed Jan 25, 2021
1 parent a388adf commit 817df81
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 33 deletions.
53 changes: 53 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ namespace TerminalAppLocalTests
TEST_METHOD(TestMultipleCommandExecuteCommandlineAction);
TEST_METHOD(TestInvalidExecuteCommandlineAction);
TEST_METHOD(TestLaunchMode);
TEST_METHOD(TestLaunchModeWithNoCommand);

private:
void _buildCommandlinesHelper(AppCommandlineArgs& appArgs,
Expand Down Expand Up @@ -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<const wchar_t*> 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<NewTabArgs>();
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<const wchar_t*> 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<NewTabArgs>();
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());
}
}
}
54 changes: 21 additions & 33 deletions src/cascadia/TerminalApp/AppCommandlineArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::string> 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;
}
Expand Down Expand Up @@ -157,6 +136,15 @@ int AppCommandlineArgs::_handleExit(const CLI::App& command, const CLI::Error& e
// - <none>
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.
Expand Down

0 comments on commit 817df81

Please sign in to comment.