Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for running a wt commandline in the curent window WITH A KEYBINDING #6537

Merged
24 commits merged into from
Jul 17, 2020
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
a0f5935
git cherry-pick 2cf69338cddb8e7bf1f50235e23a2cf9ae18791d
zadjii-msft Jun 4, 2020
3fa4687
Whoop, this works for parsing a single command! This code's horribly …
zadjii-msft Jun 4, 2020
beec8d0
do it with multiple args
zadjii-msft Jun 5, 2020
349a0e6
Cleanup for review
zadjii-msft Jun 5, 2020
fa5ecdf
`split-pane ; split-pane` at runtime doesn't work, pt1
zadjii-msft Jun 5, 2020
0622233
`split-pane ; split-pane` at runtime doesn't work, pt2
zadjii-msft Jun 5, 2020
2221764
This works but it's dirty
zadjii-msft Jun 5, 2020
030a91a
A bunch of cleanup for review
zadjii-msft Jun 5, 2020
c5613b5
more polish for review
zadjii-msft Jun 16, 2020
e1eef62
add tests
zadjii-msft Jun 16, 2020
47d5232
Merge remote-tracking branch 'origin/master' into dev/migrie/f/execut…
zadjii-msft Jul 7, 2020
fedfafa
Merge remote-tracking branch 'origin/master' into dev/migrie/f/execut…
zadjii-msft Jul 9, 2020
a4c78cd
nits from PR
zadjii-msft Jul 9, 2020
15f22c9
Don't open a new process when the cmdline is empty
zadjii-msft Jul 10, 2020
b56ea18
Merge remote-tracking branch 'origin/master' into dev/migrie/f/execut…
zadjii-msft Jul 14, 2020
87f7c79
Merge remote-tracking branch 'origin/master' into dev/migrie/f/execut…
zadjii-msft Jul 14, 2020
c290eb8
Merge branch 'dev/migrie/f/execute-order-66' of https://github.com/Mi…
zadjii-msft Jul 16, 2020
a60b68f
Merge remote-tracking branch 'origin/master' into dev/migrie/f/execut…
zadjii-msft Jul 16, 2020
6a0a270
We should, perhaps, return a warning when there's no commandline
zadjii-msft Jul 16, 2020
d57a5cc
Remove this change from earlier prototyping. It sure _wasn't_ necessary
zadjii-msft Jul 16, 2020
96f4015
don't re-use _startupActions, pass a collection in
zadjii-msft Jul 16, 2020
6eda0fe
Update src/cascadia/TerminalApp/GlobalAppSettings.cpp
zadjii-msft Jul 17, 2020
dda58f9
Merge remote-tracking branch 'origin/master' into dev/migrie/f/execut…
zadjii-msft Jul 17, 2020
6417fa9
Use the new jsonutils
zadjii-msft Jul 17, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 68 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
#include "pch.h"
#include <WexTestClass.h>

#include "../TerminalApp/TerminalPage.h"
#include "../TerminalApp/AppCommandlineArgs.h"
#include "../TerminalApp/ActionArgs.h"

using namespace WEX::Logging;
using namespace WEX::Common;
Expand Down Expand Up @@ -52,6 +54,10 @@ namespace TerminalAppLocalTests

TEST_METHOD(CheckTypos);

TEST_METHOD(TestSimpleExecuteCommandlineAction);
TEST_METHOD(TestMultipleCommandExecuteCommandlineAction);
TEST_METHOD(TestInvalidExecuteCommandlineAction);

private:
void _buildCommandlinesHelper(AppCommandlineArgs& appArgs,
const size_t expectedSubcommands,
Expand Down Expand Up @@ -1036,4 +1042,66 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(L"C:\\", myArgs.TerminalArgs().StartingDirectory());
}
}

void CommandlineTest::TestSimpleExecuteCommandlineAction()
{
auto args = winrt::make_self<implementation::ExecuteCommandlineArgs>();
args->Commandline(L"new-tab");
auto actions = implementation::TerminalPage::ConvertExecuteCommandlineToActions(*args);
VERIFY_ARE_EQUAL(1u, actions.size());
auto actionAndArgs = actions.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_TRUE(myArgs.TerminalArgs().Profile().empty());
}

void CommandlineTest::TestMultipleCommandExecuteCommandlineAction()
{
auto args = winrt::make_self<implementation::ExecuteCommandlineArgs>();
args->Commandline(L"new-tab ; split-pane");
auto actions = implementation::TerminalPage::ConvertExecuteCommandlineToActions(*args);
VERIFY_ARE_EQUAL(2u, actions.size());
{
auto actionAndArgs = actions.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_TRUE(myArgs.TerminalArgs().Profile().empty());
}
{
auto actionAndArgs = actions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::SplitPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<SplitPaneArgs>();
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_TRUE(myArgs.TerminalArgs().Profile().empty());
}
}

void CommandlineTest::TestInvalidExecuteCommandlineAction()
{
auto args = winrt::make_self<implementation::ExecuteCommandlineArgs>();
// -H and -V cannot be combined.
args->Commandline(L"split-pane -H -V");
auto actions = implementation::TerminalPage::ConvertExecuteCommandlineToActions(*args);
VERIFY_ARE_EQUAL(0u, actions.size());
}
}
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/ActionAndArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ static constexpr std::string_view ToggleFullscreenKey{ "toggleFullscreen" };
static constexpr std::string_view SetTabColorKey{ "setTabColor" };
static constexpr std::string_view OpenTabColorPickerKey{ "openTabColorPicker" };
static constexpr std::string_view RenameTabKey{ "renameTab" };
static constexpr std::string_view ExecuteCommandlineKey{ "wt" };
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
static constexpr std::string_view ToggleCommandPaletteKey{ "commandPalette" };

static constexpr std::string_view ActionKey{ "action" };
Expand Down Expand Up @@ -80,6 +81,7 @@ namespace winrt::TerminalApp::implementation
{ UnboundKey, ShortcutAction::Invalid },
{ FindKey, ShortcutAction::Find },
{ RenameTabKey, ShortcutAction::RenameTab },
{ ExecuteCommandlineKey, ShortcutAction::ExecuteCommandline },
{ ToggleCommandPaletteKey, ShortcutAction::ToggleCommandPalette },
};

Expand Down Expand Up @@ -112,6 +114,8 @@ namespace winrt::TerminalApp::implementation

{ ShortcutAction::RenameTab, winrt::TerminalApp::implementation::RenameTabArgs::FromJson },

{ ShortcutAction::ExecuteCommandline, winrt::TerminalApp::implementation::ExecuteCommandlineArgs::FromJson },

{ ShortcutAction::Invalid, nullptr },
};

Expand Down Expand Up @@ -258,6 +262,7 @@ namespace winrt::TerminalApp::implementation
{ ShortcutAction::SetTabColor, RS_(L"ResetTabColorCommandKey") },
{ ShortcutAction::OpenTabColorPicker, RS_(L"OpenTabColorPickerCommandKey") },
{ ShortcutAction::RenameTab, RS_(L"ResetTabNameCommandKey") },
{ ShortcutAction::ExecuteCommandline, RS_(L"ExecuteCommandlineCommandKey") },
{ ShortcutAction::ToggleCommandPalette, RS_(L"ToggleCommandPaletteCommandKey") },
};
}();
Expand Down
14 changes: 14 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "OpenSettingsArgs.g.cpp"
#include "SetTabColorArgs.g.cpp"
#include "RenameTabArgs.g.cpp"
#include "ExecuteCommandlineArgs.g.cpp"

#include <LibraryResources.h>

Expand Down Expand Up @@ -258,4 +259,17 @@ namespace winrt::TerminalApp::implementation
return RS_(L"ResetTabNameCommandKey");
}

winrt::hstring ExecuteCommandlineArgs::GenerateName() const
{
// "Run commandline "{_Commandline}" in this window"
if (!_Commandline.empty())
{
return winrt::hstring{
fmt::format(std::wstring_view(RS_(L"ExecuteCommandlineCommandKey")),
_Commandline.c_str())
};
}
return L"";
}

}
33 changes: 33 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "OpenSettingsArgs.g.h"
#include "SetTabColorArgs.g.h"
#include "RenameTabArgs.g.h"
#include "ExecuteCommandlineArgs.g.h"

#include "../../cascadia/inc/cppwinrt_utils.h"
#include "Utils.h"
Expand Down Expand Up @@ -531,6 +532,38 @@ namespace winrt::TerminalApp::implementation
return { *args, {} };
}
};

struct ExecuteCommandlineArgs : public ExecuteCommandlineArgsT<ExecuteCommandlineArgs>
{
ExecuteCommandlineArgs() = default;
GETSET_PROPERTY(winrt::hstring, Commandline, false);

static constexpr std::string_view CommandlineKey{ "commandline" };

public:
hstring GenerateName() const;

bool Equals(const IActionArgs& other)
{
auto otherAsUs = other.try_as<ExecuteCommandlineArgs>();
if (otherAsUs)
{
return otherAsUs->_Commandline == _Commandline;
}
return false;
};
static FromJsonResult FromJson(const Json::Value& json)
{
// LOAD BEARING: Not using make_self here _will_ break you in the future!
auto args = winrt::make_self<ExecuteCommandlineArgs>();
if (auto commandline{ json[JsonKey(CommandlineKey)] })
DHowett marked this conversation as resolved.
Show resolved Hide resolved
{
args->_Commandline = winrt::to_hstring(commandline.asString());
}
return { *args, {} };
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
}
};

}

namespace winrt::TerminalApp::factory_implementation
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.idl
Original file line number Diff line number Diff line change
Expand Up @@ -115,4 +115,9 @@ namespace TerminalApp
{
String Title { get; };
};

[default_interface] runtimeclass ExecuteCommandlineArgs : IActionArgs
{
String Commandline;
};
}
14 changes: 14 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -320,4 +320,18 @@ namespace winrt::TerminalApp::implementation
}
args.Handled(true);
}

void TerminalPage::_HandleExecuteCommandline(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& actionArgs)
{
if (const auto& realArgs = actionArgs.ActionArgs().try_as<TerminalApp::ExecuteCommandlineArgs>())
{
_startupActions = TerminalPage::ConvertExecuteCommandlineToActions(realArgs);
if (!_startupActions.empty())
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
_ProcessStartupActions(false);
actionArgs.Handled(true);
}
}
}
}
49 changes: 49 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -637,3 +637,52 @@ std::optional<winrt::TerminalApp::LaunchMode> AppCommandlineArgs::GetLaunchMode(
{
return _launchMode;
}

// Method Description:
// - Attempts to parse an array of commandline args into a list of
// commands to execute, and then parses these commands. As commands are
// successfully parsed, they will generate ShortcutActions for us to be
// able to execute. If we fail to parse any commands, we'll return the
// error code from the failure to parse that command, and stop processing
// additional commands.
// - The first arg in args should be the program name "wt" (or some variant). It
// will be ignored during parsing.
// Arguments:
// - args: an array of strings to process as a commandline. These args can contain spaces
// Return Value:
// - 0 if the commandline was successfully parsed
int AppCommandlineArgs::ParseArgs(winrt::array_view<const winrt::hstring>& args)
{
auto commands = ::TerminalApp::AppCommandlineArgs::BuildCommands(args);

for (auto& cmdBlob : commands)
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
{
// On one hand, it seems like we should be able to have one
// AppCommandlineArgs for parsing all of them, and collect the
// results one at a time.
//
// On the other hand, re-using a CLI::App seems to leave state from
// previous parsings around, so we could get mysterious behavior
// where one command affects the values of the next.
//
// From https://cliutils.github.io/CLI11/book/chapters/options.html:
// > If that option is not given, CLI11 will not touch the initial
// > value. This allows you to set up defaults by simply setting
// > your value beforehand.
//
// So we pretty much need the to either manually reset the state
// each command, or build new ones.
const auto result = ParseCommand(cmdBlob);

// If this succeeded, result will be 0. Otherwise, the caller should
// exit(result), to exit the program.
if (result != 0)
{
return result;
}
}

// If all the args were successfully parsed, we'll have some commands
// built in _appArgs, which we'll use when the application starts up.
return 0;
}
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ class TerminalApp::AppCommandlineArgs final

AppCommandlineArgs();
~AppCommandlineArgs() = default;

int ParseCommand(const Commandline& command);
int ParseArgs(winrt::array_view<const winrt::hstring>& args);

static std::vector<Commandline> BuildCommands(const std::vector<const wchar_t*>& args);
static std::vector<Commandline> BuildCommands(winrt::array_view<const winrt::hstring>& args);
Expand Down
55 changes: 4 additions & 51 deletions src/cascadia/TerminalApp/AppLogic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ namespace winrt::TerminalApp::implementation
// - <none>
// Return Value:
// - a point containing the requested dimensions in pixels.
winrt::Windows::Foundation::Point AppLogic::GetLaunchDimensions(uint32_t dpi)
winrt::Windows::Foundation::Size AppLogic::GetLaunchDimensions(uint32_t dpi)
{
if (!_loadedInitialSettings)
{
Expand Down Expand Up @@ -504,7 +504,7 @@ namespace winrt::TerminalApp::implementation
// of the height calculation here.
auto titlebar = TitlebarControl{ static_cast<uint64_t>(0) };
titlebar.Measure({ SHRT_MAX, SHRT_MAX });
proposedSize.Y += (titlebar.DesiredSize().Height) * scale;
proposedSize.Height += (titlebar.DesiredSize().Height) * scale;
}
else if (_settings->GlobalSettings().AlwaysShowTabs())
{
Expand All @@ -519,7 +519,7 @@ namespace winrt::TerminalApp::implementation
// For whatever reason, there's about 6px of unaccounted-for space
// in the application. I couldn't tell you where these 6px are
// coming from, but they need to be included in this math.
proposedSize.Y += (tabControl.DesiredSize().Height + 6) * scale;
proposedSize.Width += (tabControl.DesiredSize().Height + 6) * scale;
}

return proposedSize;
Expand Down Expand Up @@ -974,7 +974,7 @@ namespace winrt::TerminalApp::implementation
// or 0. (see AppLogic::_ParseArgs)
int32_t AppLogic::SetStartupCommandline(array_view<const winrt::hstring> args)
{
const auto result = _ParseArgs(args);
const auto result = _appArgs.ParseArgs(args);
if (result == 0)
{
_appArgs.ValidateStartupCommands();
Expand All @@ -984,53 +984,6 @@ namespace winrt::TerminalApp::implementation
return result;
}

// Method Description:
// - Attempts to parse an array of commandline args into a list of
// commands to execute, and then parses these commands. As commands are
// successfully parsed, they will generate ShortcutActions for us to be
// able to execute. If we fail to parse any commands, we'll return the
// error code from the failure to parse that command, and stop processing
// additional commands.
// Arguments:
// - args: an array of strings to process as a commandline. These args can contain spaces
// Return Value:
// - 0 if the commandline was successfully parsed
int AppLogic::_ParseArgs(winrt::array_view<const hstring>& args)
{
auto commands = ::TerminalApp::AppCommandlineArgs::BuildCommands(args);

for (auto& cmdBlob : commands)
{
// On one hand, it seems like we should be able to have one
// AppCommandlineArgs for parsing all of them, and collect the
// results one at a time.
//
// On the other hand, re-using a CLI::App seems to leave state from
// previous parsings around, so we could get mysterious behavior
// where one command affects the values of the next.
//
// From https://cliutils.github.io/CLI11/book/chapters/options.html:
// > If that option is not given, CLI11 will not touch the initial
// > value. This allows you to set up defaults by simply setting
// > your value beforehand.
//
// So we pretty much need the to either manually reset the state
// each command, or build new ones.
const auto result = _appArgs.ParseCommand(cmdBlob);

// If this succeeded, result will be 0. Otherwise, the caller should
// exit(result), to exit the program.
if (result != 0)
{
return result;
}
}

// If all the args were successfully parsed, we'll have some commands
// built in _appArgs, which we'll use when the application starts up.
return 0;
}

// Method Description:
// - If there were any errors parsing the commandline that was used to
// initialize the terminal, this will return a string containing that
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppLogic.h
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ namespace winrt::TerminalApp::implementation
winrt::hstring ApplicationDisplayName() const;
winrt::hstring ApplicationVersion() const;

Windows::Foundation::Point GetLaunchDimensions(uint32_t dpi);
Windows::Foundation::Size GetLaunchDimensions(uint32_t dpi);
winrt::Windows::Foundation::Point GetLaunchInitialPositions(int32_t defaultInitialX, int32_t defaultInitialY);
winrt::Windows::UI::Xaml::ElementTheme GetRequestedTheme();
LaunchMode GetLaunchMode();
Expand Down
2 changes: 1 addition & 1 deletion src/cascadia/TerminalApp/AppLogic.idl
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ namespace TerminalApp
String ApplicationDisplayName { get; };
String ApplicationVersion { get; };

Windows.Foundation.Point GetLaunchDimensions(UInt32 dpi);
Windows.Foundation.Size GetLaunchDimensions(UInt32 dpi);
Windows.Foundation.Point GetLaunchInitialPositions(Int32 defaultInitialX, Int32 defaultInitialY);
Windows.UI.Xaml.ElementTheme GetRequestedTheme();
LaunchMode GetLaunchMode();
Expand Down
Loading