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 all 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
19 changes: 19 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"openTabColorPicker",
"renameTab",
"commandPalette",
"wt",
"unbound"
],
"type": "string"
Expand Down Expand Up @@ -280,6 +281,23 @@
}
]
},
"WtAction": {
"description": "Arguments corresponding to a wt Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "wt" },
"commandline": {
"type": "string",
"default": "",
"description": "a `wt` commandline to run in the current window"
}
}
}
],
"required": [ "commandline" ]
},
"Keybinding": {
"additionalProperties": false,
"properties": {
Expand All @@ -296,6 +314,7 @@
{ "$ref": "#/definitions/SplitPaneAction" },
{ "$ref": "#/definitions/OpenSettingsAction" },
{ "$ref": "#/definitions/SetTabColorAction" },
{ "$ref": "#/definitions/WtAction" },
{ "type": "null" }
]
},
Expand Down
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 @@ -1067,4 +1073,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());
}
}
53 changes: 53 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ namespace TerminalAppLocalTests

TEST_METHOD(ValidateKeybindingsWarnings);

TEST_METHOD(ValidateExecuteCommandlineWarning);

TEST_METHOD(ValidateLegacyGlobalsWarning);

TEST_METHOD(TestTrailingCommas);
Expand Down Expand Up @@ -2254,6 +2256,57 @@ namespace TerminalAppLocalTests
VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_warnings.at(3));
}

void SettingsTests::ValidateExecuteCommandlineWarning()
{
Log::Comment(L"This test is affected by GH#6949, so we're just skipping it for now.");
Log::Result(WEX::Logging::TestResults::Skipped);
return;

// const std::string badSettings{ R"(
// {
// "defaultProfile": "{6239a42c-2222-49a3-80bd-e8fdd045185c}",
// "profiles": [
// {
// "name" : "profile0",
// "guid": "{6239a42c-2222-49a3-80bd-e8fdd045185c}"
// },
// {
// "name" : "profile1",
// "guid": "{6239a42c-3333-49a3-80bd-e8fdd045185c}"
// }
// ],
// "keybindings": [
// { "name":null, "command": { "action": "wt" }, "keys": [ "ctrl+a" ] },
// { "name":null, "command": { "action": "wt", "commandline":"" }, "keys": [ "ctrl+b" ] },
// { "name":null, "command": { "action": "wt", "commandline":null }, "keys": [ "ctrl+c" ] }
// ]
// })" };

// const auto settingsObject = VerifyParseSucceeded(badSettings);

// auto settings = CascadiaSettings::FromJson(settingsObject);

// VERIFY_ARE_EQUAL(0u, settings->_globals._keybindings->_keyShortcuts.size());

// for (const auto& warning : settings->_globals._keybindingsWarnings)
// {
// Log::Comment(NoThrowString().Format(
// L"warning:%d", warning));
// }
// VERIFY_ARE_EQUAL(3u, settings->_globals._keybindingsWarnings.size());
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_globals._keybindingsWarnings.at(0));
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_globals._keybindingsWarnings.at(1));
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_globals._keybindingsWarnings.at(2));

// settings->_ValidateKeybindings();

// VERIFY_ARE_EQUAL(4u, settings->_warnings.size());
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::AtLeastOneKeybindingWarning, settings->_warnings.at(0));
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_warnings.at(1));
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_warnings.at(2));
// VERIFY_ARE_EQUAL(::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter, settings->_warnings.at(3));
}

void SettingsTests::ValidateLegacyGlobalsWarning()
{
const std::string badSettings{ R"(
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/ActionAndArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ static constexpr std::string_view ToggleAlwaysOnTopKey{ "toggleAlwaysOnTop" };
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 @@ -89,6 +90,7 @@ namespace winrt::TerminalApp::implementation
{ UnboundKey, ShortcutAction::Invalid },
{ FindKey, ShortcutAction::Find },
{ RenameTabKey, ShortcutAction::RenameTab },
{ ExecuteCommandlineKey, ShortcutAction::ExecuteCommandline },
{ ToggleCommandPaletteKey, ShortcutAction::ToggleCommandPalette },
};

Expand Down Expand Up @@ -121,6 +123,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 @@ -268,6 +272,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"";
}

}
34 changes: 34 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 @@ -387,6 +388,39 @@ namespace winrt::TerminalApp::implementation
return { *args, {} };
}
};

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

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>();
JsonUtils::GetValueForKey(json, CommandlineKey, args->_Commandline);
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
if (args->_Commandline.empty())
{
return { nullptr, { ::TerminalApp::SettingsLoadWarnings::MissingRequiredParameter } };
}
return { *args, {} };
}
};

}

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;
};
}
17 changes: 17 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ using namespace winrt::Windows::ApplicationModel::DataTransfer;
using namespace winrt::Windows::UI::Xaml;
using namespace winrt::Windows::UI::Text;
using namespace winrt::Windows::UI::Core;
using namespace winrt::Windows::Foundation::Collections;
using namespace winrt::Windows::System;
using namespace winrt::Microsoft::Terminal;
using namespace winrt::Microsoft::Terminal::Settings;
Expand Down Expand Up @@ -334,4 +335,20 @@ 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>())
{
auto actions = winrt::single_threaded_vector<winrt::TerminalApp::ActionAndArgs>(std::move(
TerminalPage::ConvertExecuteCommandlineToActions(realArgs)));

if (_startupActions.Size() != 0)
{
actionArgs.Handled(true);
_ProcessStartupActions(actions, false);
}
}
}
}
Loading