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

Fixed #3799: Introduce sendInput command #7249

Merged
4 commits merged into from
Aug 12, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
1 change: 1 addition & 0 deletions doc/cascadia/SettingsSchema.md
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ For commands with arguments:
| `scrollUp` | Move the screen up. | | | |
| `scrollUpPage` | Move the screen up a whole page. | | | |
| `scrollDownPage` | Move the screen down a whole page. | | | |
| `sendInput` | Sends some text input to the shell. | `input` | string | The text input to feed into the shell.<br>ANSI escape sequences may be used. Escape codes like `\x1b` must be written as `\u001b`.<br>For instance the input `"text\n"` will write "text" followed by a newline. `"\u001b[D"` will behave as if the left arrow button had been pressed. |
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this classify as "Documentation updated"? Do I need to submit a PR to the docs repo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, you do!

| `splitPane` | Halve the size of the active pane and open another. Without any arguments, this will open the default profile in the new pane. | 1. `split`*<br>2. `commandLine`<br>3. `startingDirectory`<br>4. `tabTitle`<br>5. `index`<br>6. `profile`<br>7. `splitMode` | 1. `vertical`, `horizontal`, `auto`<br>2. string<br>3. string<br>4. string<br>5. integer<br>6. string<br>7. string | 1. How the pane will split. `auto` will split in the direction that provides the most surface area.<br>2. Executable run within the pane.<br>3. Directory in which the pane will open.<br>4. Title of the tab when the new pane is focused.<br>5. Profile that will open based on its position in the dropdown (starting at 0).<br>6. Profile that will open based on its GUID or name.<br>7. Controls how the pane splits. Only accepts `duplicate` which will duplicate the focused pane's profile into a new pane. |
| `switchToTab` | Open a specific tab depending on index. | `index`* | integer | Tab that will open based on its position in the tab bar (starting at 0). |
| `toggleFullscreen` | Switch between fullscreen and default window sizes. | | | |
Expand Down
24 changes: 24 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@
"scrollDownPage",
"scrollUp",
"scrollUpPage",
"sendInput",
"splitPane",
"switchToTab",
"toggleFocusMode",
Expand Down Expand Up @@ -230,6 +231,28 @@
],
"required": [ "direction" ]
},
"SendInputAction": {
"description": "Arguments corresponding to a Send Input Action",
"allOf": [
{
"$ref": "#/definitions/ShortcutAction"
},
{
"properties": {
"action": {
"type": "string",
"pattern": "sendInput"
},
"input": {
"type": "string",
"default": "",
"description": "The text input to feed into the shell. ANSI escape sequences may be used. Escape codes like \\x1b must be written as \\u001b."
}
}
}
],
"required": [ "input" ]
},
"SplitPaneAction": {
"description": "Arguments corresponding to a Split Pane Action",
"allOf": [
Expand Down Expand Up @@ -390,6 +413,7 @@
{ "$ref": "#/definitions/SwitchToTabAction" },
{ "$ref": "#/definitions/MoveFocusAction" },
{ "$ref": "#/definitions/ResizePaneAction" },
{ "$ref": "#/definitions/SendInputAction" },
{ "$ref": "#/definitions/SplitPaneAction" },
{ "$ref": "#/definitions/OpenSettingsAction" },
{ "$ref": "#/definitions/SetTabColorAction" },
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 @@ -27,6 +27,7 @@ static constexpr std::string_view ScrolluppageKey{ "scrollUpPage" };
static constexpr std::string_view ScrolldownpageKey{ "scrollDownPage" };
static constexpr std::string_view SwitchToTabKey{ "switchToTab" };
static constexpr std::string_view OpenSettingsKey{ "openSettings" }; // TODO GH#2557: Add args for OpenSettings
static constexpr std::string_view SendInputKey{ "sendInput" };
static constexpr std::string_view SplitPaneKey{ "splitPane" };
static constexpr std::string_view TogglePaneZoomKey{ "togglePaneZoom" };
static constexpr std::string_view ResizePaneKey{ "resizePane" };
Expand Down Expand Up @@ -90,6 +91,7 @@ namespace winrt::TerminalApp::implementation
{ ToggleFocusModeKey, ShortcutAction::ToggleFocusMode },
{ ToggleFullscreenKey, ShortcutAction::ToggleFullscreen },
{ ToggleAlwaysOnTopKey, ShortcutAction::ToggleAlwaysOnTop },
{ SendInputKey, ShortcutAction::SendInput },
{ SplitPaneKey, ShortcutAction::SplitPane },
{ TogglePaneZoomKey, ShortcutAction::TogglePaneZoom },
{ SetTabColorKey, ShortcutAction::SetTabColor },
Expand Down Expand Up @@ -125,6 +127,8 @@ namespace winrt::TerminalApp::implementation

{ ShortcutAction::AdjustFontSize, winrt::TerminalApp::implementation::AdjustFontSizeArgs::FromJson },

{ ShortcutAction::SendInput, winrt::TerminalApp::implementation::SendInputArgs::FromJson },

{ ShortcutAction::SplitPane, winrt::TerminalApp::implementation::SplitPaneArgs::FromJson },

{ ShortcutAction::OpenSettings, winrt::TerminalApp::implementation::OpenSettingsArgs::FromJson },
Expand Down Expand Up @@ -284,6 +288,7 @@ namespace winrt::TerminalApp::implementation
{ ShortcutAction::ToggleFocusMode, RS_(L"ToggleFocusModeCommandKey") },
{ ShortcutAction::ToggleFullscreen, RS_(L"ToggleFullscreenCommandKey") },
{ ShortcutAction::ToggleAlwaysOnTop, RS_(L"ToggleAlwaysOnTopCommandKey") },
{ ShortcutAction::SendInput, RS_(L"SendInputCommandKey") },
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should be mapped to L"", so that we don't accidentally generate a name for a sendInput action that doesn't have a value for input

see for example: CloseOtherTabs

{ ShortcutAction::SplitPane, RS_(L"SplitPaneCommandKey") },
{ ShortcutAction::TogglePaneZoom, RS_(L"TogglePaneZoomCommandKey") },
{ ShortcutAction::Invalid, L"" },
Expand Down
11 changes: 11 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "ResizePaneArgs.g.cpp"
#include "MoveFocusArgs.g.cpp"
#include "AdjustFontSizeArgs.g.cpp"
#include "SendInputArgs.g.cpp"
#include "SplitPaneArgs.g.cpp"
#include "OpenSettingsArgs.g.cpp"
#include "SetColorSchemeArgs.g.cpp"
Expand Down Expand Up @@ -167,6 +168,16 @@ namespace winrt::TerminalApp::implementation
}
}

winrt::hstring SendInputArgs::GenerateName() const
{
// The string will be similar to the following:
// * "Send Input: ...input..."

return winrt::hstring{
fmt::format(L"{}: {}", RS_(L"SendInputCommandKey"), _Input)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typically, it's best practice to have just two entirely different resources in Resources.resw for something like this.

In this case though, I don't think we need the version without the argument at all, we could probably get away with a single resource for the formatted version:

  <data name="SendInputCommandKey" xml:space="preserve">
    <value>Send Input: "{0}"</value>
    <comment>{0} will be replaced with a string of input as defined by the user.</comment>
  </data>

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm suddenly worried about dropping the user's input right into a TextBlock. Should we transform the control characters <0x20 to their "control pictures" equivalents? It's as easy as adding 0x2400 to them!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what we do in the debug tap ... which I can't get a screenshot of because we broke it o_O

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps my right alt key no longer works. This would come as no big surprise.

Copy link
Member

@zadjii-msft zadjii-msft Aug 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we transform the control characters <0x20 to their "control pictures" equivalents? It's as easy as adding 0x2400 to them!

I'm hugely in favor of this actually. Raw control characters might definitely will break the text block

};
}

winrt::hstring SplitPaneArgs::GenerateName() const
{
// The string will be similar to the following:
Expand Down
28 changes: 28 additions & 0 deletions src/cascadia/TerminalApp/ActionArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "ResizePaneArgs.g.h"
#include "MoveFocusArgs.g.h"
#include "AdjustFontSizeArgs.g.h"
#include "SendInputArgs.g.h"
#include "SplitPaneArgs.g.h"
#include "OpenSettingsArgs.g.h"
#include "SetColorSchemeArgs.g.h"
Expand Down Expand Up @@ -270,6 +271,33 @@ namespace winrt::TerminalApp::implementation
}
};

struct SendInputArgs : public SendInputArgsT<SendInputArgs>
{
SendInputArgs() = default;
GETSET_PROPERTY(winrt::hstring, Input, L"");

static constexpr std::string_view InputKey{ "input" };

public:
hstring GenerateName() const;

bool Equals(const IActionArgs& other)
{
if (auto otherAsUs = other.try_as<SendInputArgs>(); otherAsUs)
{
return otherAsUs->_Input == _Input;
}
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<SendInputArgs>();
JsonUtils::GetValueForKey(json, InputKey, args->_Input);
lhecker marked this conversation as resolved.
Show resolved Hide resolved
return { *args, {} };
}
};

struct SplitPaneArgs : public SplitPaneArgsT<SplitPaneArgs>
{
SplitPaneArgs() = default;
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 @@ -94,6 +94,11 @@ namespace TerminalApp
Int32 Delta { get; };
};

[default_interface] runtimeclass SendInputArgs : IActionArgs
{
String Input { get; };
};

[default_interface] runtimeclass SplitPaneArgs : IActionArgs
{
SplitState SplitStyle { get; };
Expand Down
15 changes: 15 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,21 @@ namespace winrt::TerminalApp::implementation
args.Handled(true);
}

void TerminalPage::_HandleSendInput(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
if (args == nullptr)
{
args.Handled(false);
}
else if (const auto& realArgs = args.ActionArgs().try_as<TerminalApp::SendInputArgs>())
{
const auto termControl = _GetActiveControl();
termControl.SendInput(realArgs.Input());
args.Handled(true);
}
}

void TerminalPage::_HandleSplitPane(const IInspectable& /*sender*/,
const TerminalApp::ActionEventArgs& args)
{
Expand Down
3 changes: 3 additions & 0 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,9 @@
<data name="NewTabCommandKey" xml:space="preserve">
<value>New tab</value>
</data>
<data name="SendInputCommandKey" xml:space="preserve">
zadjii-msft marked this conversation as resolved.
Show resolved Hide resolved
<value>Send Input</value>
</data>
<data name="SplitPaneCommandKey" xml:space="preserve">
<value>Split pane</value>
</data>
Expand Down
6 changes: 6 additions & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,12 @@ namespace winrt::TerminalApp::implementation
break;
}

case ShortcutAction::SendInput:
{
_SendInputHandlers(*this, *eventArgs);
break;
}

case ShortcutAction::SplitVertical:
case ShortcutAction::SplitHorizontal:
case ShortcutAction::SplitPane:
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ namespace winrt::TerminalApp::implementation
TYPED_EVENT(SwitchToTab, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(NextTab, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(PrevTab, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(SendInput, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(SplitPane, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(TogglePaneZoom, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
TYPED_EVENT(AdjustFontSize, TerminalApp::ShortcutActionDispatch, TerminalApp::ActionEventArgs);
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/ShortcutActionDispatch.idl
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ namespace TerminalApp
PrevTab,
SplitVertical,
SplitHorizontal,
SendInput,
SplitPane,
TogglePaneZoom,
SwitchToTab,
Expand Down Expand Up @@ -71,6 +72,7 @@ namespace TerminalApp
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> SwitchToTab;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> NextTab;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> PrevTab;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> SendInput;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> SplitPane;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> TogglePaneZoom;
event Windows.Foundation.TypedEventHandler<ShortcutActionDispatch, ActionEventArgs> AdjustFontSize;
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,7 @@ namespace winrt::TerminalApp::implementation
_actionDispatch->ScrollDown({ this, &TerminalPage::_HandleScrollDown });
_actionDispatch->NextTab({ this, &TerminalPage::_HandleNextTab });
_actionDispatch->PrevTab({ this, &TerminalPage::_HandlePrevTab });
_actionDispatch->SendInput({ this, &TerminalPage::_HandleSendInput });
_actionDispatch->SplitPane({ this, &TerminalPage::_HandleSplitPane });
_actionDispatch->TogglePaneZoom({ this, &TerminalPage::_HandleTogglePaneZoom });
_actionDispatch->ScrollUpPage({ this, &TerminalPage::_HandleScrollUpPage });
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/TerminalPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ namespace winrt::TerminalApp::implementation
void _HandleScrollDown(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
void _HandleNextTab(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
void _HandlePrevTab(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
void _HandleSendInput(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
void _HandleSplitPane(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
void _HandleTogglePaneZoom(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
void _HandleScrollUpPage(const IInspectable& sender, const TerminalApp::ActionEventArgs& args);
Expand Down
14 changes: 13 additions & 1 deletion src/cascadia/TerminalControl/TermControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,18 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
}
}
}

// Method Description:
// - Writes the given sequence as input to the active terminal connection,
// Arguments:
// - wstr: the string of characters to write to the terminal connection.
// Return Value:
// - <none>
void TermControl::SendInput(const winrt::hstring& input)
{
_SendInputToConnection(input);
}

void TermControl::ToggleRetroEffect()
{
auto lock = _terminal->LockForWriting();
Expand Down Expand Up @@ -1769,7 +1781,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
// - wstr: the string of characters to write to the terminal connection.
// Return Value:
// - <none>
void TermControl::_SendInputToConnection(const std::wstring& wstr)
void TermControl::_SendInputToConnection(const winrt::param::hstring& wstr)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unfortunately ☹️ we can't actually use the winrt::param namespace even though it is literally the best namespace for taking in anything that can convert to hstring

We might have to prefer std::wstring_view, which can be natively converted to an hstring (though it could also crash because hstring expects string_views to be null-terminated)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I've discussed this with the cppwinrt folks before. param::hstring does everything we might possibly want with regards to automatic promotion to hstring, but it's not part of the public interface :/)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not entirely sure I understand... Am I supposed to not use winrt::param because it's a private interface, or is there another technical reason?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, just because it is a private namespace.

{
_connection.WriteInput(wstr);
}
Expand Down
3 changes: 2 additions & 1 deletion src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation
void AdjustFontSize(int fontSizeDelta);
void ResetFontSize();

void SendInput(const winrt::hstring& input);
void ToggleRetroEffect();

winrt::fire_and_forget RenderEngineSwapChainChanged();
Expand Down Expand Up @@ -221,7 +222,7 @@ namespace winrt::Microsoft::Terminal::TerminalControl::implementation

void _CursorTimerTick(Windows::Foundation::IInspectable const& sender, Windows::Foundation::IInspectable const& e);
void _SetEndSelectionPointAtCursor(Windows::Foundation::Point const& cursorPosition);
void _SendInputToConnection(const std::wstring& wstr);
void _SendInputToConnection(const winrt::param::hstring& wstr);
void _SendPastedTextToConnection(const std::wstring& wstr);
void _SwapChainSizeChanged(Windows::Foundation::IInspectable const& sender, Windows::UI::Xaml::SizeChangedEventArgs const& e);
void _SwapChainScaleChanged(Windows::UI::Xaml::Controls::SwapChainPanel const& sender, Windows::Foundation::IInspectable const& args);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ namespace Microsoft.Terminal.TerminalControl
void AdjustFontSize(Int32 fontSizeDelta);
void ResetFontSize();

void SendInput(String input);
void ToggleRetroEffect();

Windows.Foundation.IReference<Windows.UI.Color> TabColor { get; };
Expand Down