Skip to content

Commit

Permalink
Add support for running a wt commandline in the curent window WITH …
Browse files Browse the repository at this point in the history
…A KEYBINDING (#6537)

## Summary of the Pull Request

Adds a execute commandline action (`wt`), which lets a user bind a key to a specific `wt` commandline. This commandline will get parsed and run _in the current window_. 

## References

* Related to #4472 
* Related to #5400 - I need this for the commandline mode of the Command Palette
* Related to #5970

## PR Checklist
* [x] Closes oh, there's not actually an issue for this.
* [x] I work here
* [x] Tests added/passed
* [ ] Requires documentation to be updated - yes it does

## Detailed Description of the Pull Request / Additional comments

One important part of this change concerns how panes are initialized at runtime. We've had some persistent trouble with initializing multiple panes, because they rely on knowing how big they'll actually be, to be able to determine if they can split again. 

We previously worked around this by ignoring the size check when we were in "startup", processing an initial commandline. This PR however requires us to be able to know the initial size of a pane at runtime, but before the parents have necessarily been added to the tree, or had their renderer's set up.

This led to the development of `Pane::PreCalculateCanSplit`, which is very highly similar to `Pane::PreCalculateAutoSplit`. This method attempts to figure out how big a pane _will_ take, before the parent has necessarily laid out. 

This also involves a small change to `TermControl`, because if its renderer hasn't been set up yet, it'll always think the font is `{0, fontHeight}`, which will let the Terminal keep splitting in the x direction. This change also makes the TermControl set up a renderer to get the real font size when it hasn't yet been initialized.

## Validation Steps Performed

This was what the json blob I was using for testing evolved into

```json
        {
            "command": {
                "action":"wt",
                "commandline": "new-tab cmd.exe /k #work 15 ; split-pane cmd.exe /k #work 15 ; split-pane cmd.exe /k media-commandline ; new-tab powershell dev\\symbols.ps1 ; new-tab -p \"Ubuntu\" ; new-tab -p \"haunter.gif\" ; focus-tab -t 0",

            },
            "keys": ["ctrl+shift+n"]
        }
```

I also added some tests.

# TODO
* [x] Creating a `{ "command": "wt" }` action without a commandline will spawn a new `wt.exe` process?
  - Probably should just do nothing for the empty string
  • Loading branch information
zadjii-msft authored Jul 17, 2020
1 parent 3a91fc0 commit d0ff5f6
Show file tree
Hide file tree
Showing 29 changed files with 582 additions and 115 deletions.
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" };
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);
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

0 comments on commit d0ff5f6

Please sign in to comment.