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 focusPane action, focus-pane subcommand #10142

Merged
8 commits merged into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from 7 commits
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
18 changes: 18 additions & 0 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@
"duplicateTab",
"find",
"findMatch",
"focusPane",
"identifyWindow",
"identifyWindows",
"moveFocus",
Expand Down Expand Up @@ -798,6 +799,22 @@
}
]
},
"FocusPaneAction": {
"description": "Arguments corresponding to a focusPane Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "focusPane" },
"id": {
"type": "string",
"default": "",
"description": "The ID of the pane to focus"
}
}
}
]
},
"Keybinding": {
"additionalProperties": false,
"properties": {
Expand Down Expand Up @@ -828,6 +845,7 @@
{ "$ref": "#/definitions/PrevTabAction" },
{ "$ref": "#/definitions/RenameTabAction" },
{ "$ref": "#/definitions/RenameWindowAction" },
{ "$ref": "#/definitions/FocusPaneAction" },
{ "type": "null" }
]
},
Expand Down
103 changes: 103 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ namespace TerminalAppLocalTests
TEST_METHOD(ParseFocusTabArgs);
TEST_METHOD(ParseMoveFocusArgs);
TEST_METHOD(ParseArgumentsWithParsingTerminators);
TEST_METHOD(ParseFocusPaneArgs);

TEST_METHOD(ParseNoCommandIsNewTab);

Expand Down Expand Up @@ -1206,6 +1207,108 @@ namespace TerminalAppLocalTests
}
}

void CommandlineTest::ParseFocusPaneArgs()
{
BEGIN_TEST_METHOD_PROPERTIES()
TEST_METHOD_PROPERTY(L"Data:useShortForm", L"{false, true}")
END_TEST_METHOD_PROPERTIES()

INIT_TEST_PROPERTY(bool, useShortForm, L"If true, use `fp` instead of `focus-pane`");
const wchar_t* subcommand = useShortForm ? L"fp" : L"focus-pane";

{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand };
Log::Comment(NoThrowString().Format(
L"Just the subcommand, without a target, should fail."));

_buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands);
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"left" };

Log::Comment(NoThrowString().Format(
L"focus-pane without a target should fail."));
_buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands);
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"1" };

Log::Comment(NoThrowString().Format(
L"focus-pane without a target should fail."));
_buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands);
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"--target", L"0" };
_buildCommandlinesHelper(appArgs, 1u, rawCommands);

VERIFY_ARE_EQUAL(2u, appArgs._startupActions.size());

// The first action is going to always be a new-tab action
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::FocusPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<FocusPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(0u, myArgs.Id());
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"-t", L"100" };
_buildCommandlinesHelper(appArgs, 1u, rawCommands);

VERIFY_ARE_EQUAL(2u, appArgs._startupActions.size());

// The first action is going to always be a new-tab action
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());

auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::FocusPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<FocusPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(100u, myArgs.Id());
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"--target", L"-1" };
Log::Comment(NoThrowString().Format(
L"focus-pane with an invalid target should fail."));
_buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands);
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", L"move-focus", L"left", L";", subcommand, L"-t", L"1" };
_buildCommandlinesHelper(appArgs, 2u, rawCommands);

VERIFY_ARE_EQUAL(3u, appArgs._startupActions.size());

// The first action is going to always be a new-tab action
VERIFY_ARE_EQUAL(ShortcutAction::NewTab, appArgs._startupActions.at(0).Action());
{
auto actionAndArgs = appArgs._startupActions.at(1);
VERIFY_ARE_EQUAL(ShortcutAction::MoveFocus, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MoveFocusArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.FocusDirection());
}
{
auto actionAndArgs = appArgs._startupActions.at(2);
VERIFY_ARE_EQUAL(ShortcutAction::FocusPane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<FocusPaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(1u, myArgs.Id());
}
}
}

void CommandlineTest::ValidateFirstCommandIsNewTab()
{
AppCommandlineArgs appArgs{};
Expand Down
17 changes: 17 additions & 0 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -790,4 +790,21 @@ namespace winrt::TerminalApp::implementation
// if it wasn't bound at all.
args.Handled(false);
}

void TerminalPage::_HandleFocusPane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (args)
{
if (const auto& realArgs = args.ActionArgs().try_as<FocusPaneArgs>())
{
const auto paneId = realArgs.Id();
if (const auto activeTab{ _GetFocusedTabImpl() })
{
_UnZoomIfNeeded();
args.Handled(activeTab->FocusPane(paneId));
}
}
}
}
}
48 changes: 46 additions & 2 deletions src/cascadia/TerminalApp/AppCommandlineArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@
// Licensed under the MIT license.

#include "pch.h"
#include "AppLogic.h"
#include "AppCommandlineArgs.h"
#include "../types/inc/utils.hpp"
#include <LibraryResources.h>

using namespace winrt::TerminalApp;
using namespace winrt::Microsoft::Terminal::Settings::Model;
using namespace TerminalApp;

Expand Down Expand Up @@ -194,6 +192,7 @@ void AppCommandlineArgs::_buildParser()
_buildSplitPaneParser();
_buildFocusTabParser();
_buildMoveFocusParser();
_buildFocusPaneParser();
}

// Method Description:
Expand Down Expand Up @@ -399,6 +398,46 @@ void AppCommandlineArgs::_buildMoveFocusParser()
setupSubcommand(_moveFocusShort);
}

// Method Description:
// - Adds the `focus-pane` subcommand and related options to the commandline parser.
// - Additionally adds the `fp` subcommand, which is just a shortened version of `focus-pane`
// Arguments:
// - <none>
// Return Value:
// - <none>
void AppCommandlineArgs::_buildFocusPaneParser()
{
_focusPaneCommand = _app.add_subcommand("focus-pane", RS_A(L"CmdFocusPaneDesc"));
_focusPaneShort = _app.add_subcommand("fp", RS_A(L"CmdFPDesc"));

auto setupSubcommand = [this](auto* subcommand) {
auto* targetOpt = subcommand->add_option("-t,--target",
_focusPaneTarget,
RS_A(L"CmdFocusPaneTargetArgDesc"));
targetOpt->required();
targetOpt->check(CLI::NonNegativeNumber);
// When ParseCommand is called, if this subcommand was provided, this
// callback function will be triggered on the same thread. We can be sure
// that `this` will still be safe - this function just lets us know this
// command was parsed.
subcommand->callback([&, this]() {
// Build the action from the values we've parsed on the commandline.
ActionAndArgs focusPaneAction{};
DHowett marked this conversation as resolved.
Show resolved Hide resolved

if (_focusPaneTarget >= 0)
{
DHowett marked this conversation as resolved.
Show resolved Hide resolved
focusPaneAction.Action(ShortcutAction::FocusPane);
FocusPaneArgs args{ static_cast<uint32_t>(_focusPaneTarget) };
focusPaneAction.Args(args);
_startupActions.push_back(focusPaneAction);
}
});
};

setupSubcommand(_focusPaneCommand);
setupSubcommand(_focusPaneShort);
}

// Method Description:
// - Add the `NewTerminalArgs` parameters to the given subcommand. This enables
// that subcommand to support all the properties in a NewTerminalArgs.
Expand Down Expand Up @@ -536,6 +575,8 @@ bool AppCommandlineArgs::_noCommandsProvided()
*_focusTabShort ||
*_moveFocusCommand ||
*_moveFocusShort ||
*_focusPaneCommand ||
*_focusPaneShort ||
*_newPaneShort.subcommand ||
*_newPaneCommand.subcommand);
}
Expand Down Expand Up @@ -567,6 +608,9 @@ void AppCommandlineArgs::_resetStateToDefault()
_focusPrevTab = false;

_moveFocusDirection = FocusDirection::None;

_focusPaneTarget = -1;

// DON'T clear _launchMode here! This will get called once for every
// subcommand, so we don't want `wt -F new-tab ; split-pane` clearing out
// the "global" fullscreen flag (-F).
Expand Down
5 changes: 5 additions & 0 deletions src/cascadia/TerminalApp/AppCommandlineArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ class TerminalApp::AppCommandlineArgs final
CLI::App* _focusTabShort;
CLI::App* _moveFocusCommand;
CLI::App* _moveFocusShort;
CLI::App* _focusPaneCommand;
CLI::App* _focusPaneShort;

// Are you adding a new sub-command? Make sure to update _noCommandsProvided!

Expand All @@ -105,6 +107,8 @@ class TerminalApp::AppCommandlineArgs final
int _focusTabIndex{ -1 };
bool _focusNextTab{ false };
bool _focusPrevTab{ false };

int _focusPaneTarget{ -1 };
// Are you adding more args here? Make sure to reset them in _resetStateToDefault

const Commandline* _currentCommandline{ nullptr };
Expand All @@ -124,6 +128,7 @@ class TerminalApp::AppCommandlineArgs final
void _buildSplitPaneParser();
void _buildFocusTabParser();
void _buildMoveFocusParser();
void _buildFocusPaneParser();
bool _noCommandsProvided();
void _resetStateToDefault();
int _handleExit(const CLI::App& command, const CLI::Error& e);
Expand Down
47 changes: 29 additions & 18 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -597,18 +597,25 @@ void Pane::_FocusFirstChild()
{
if (_IsLeaf())
{
if (_root.ActualWidth() == 0 && _root.ActualHeight() == 0)
{
// When these sizes are 0, then the pane might still be in startup,
// and doesn't yet have a real size. In that case, the control.Focus
// event won't be handled until _after_ the startup events are all
// processed. This will lead to the Tab not being notified that the
// focus moved to a different Pane.
//
// In that scenario, trigger the event manually here, to correctly
// inform the Tab that we're now focused.
_GotFocusHandlers(shared_from_this());
}
// Originally, we would only raise a GotFocus event here when:
//
// if (_root.ActualWidth() == 0 && _root.ActualHeight() == 0)
//
// When these sizes are 0, then the pane might still be in startup,
// and doesn't yet have a real size. In that case, the control.Focus
// event won't be handled until _after_ the startup events are all
// processed. This will lead to the Tab not being notified that the
// focus moved to a different Pane.
//
// However, with the ability to execute multiple actions at a time, in
// already existing windows, we need to always raise this event manually
// here, to correctly inform the Tab that we're now focused. This will
// take care of commandlines like:
//
// `wtd -w 0 mf down ; sp`
// `wtd -w 0 fp -t 1 ; sp`

_GotFocusHandlers(shared_from_this());

_control.Focus(FocusState::Programmatic);
}
Expand Down Expand Up @@ -1564,7 +1571,7 @@ void Pane::Restore(std::shared_ptr<Pane> zoomedPane)
// otherwise the ID value will not make sense (leaves have IDs, parents do not)
// Return Value:
// - The ID of this pane
std::optional<uint16_t> Pane::Id() noexcept
std::optional<uint32_t> Pane::Id() noexcept
{
return _id;
}
Expand All @@ -1574,7 +1581,7 @@ std::optional<uint16_t> Pane::Id() noexcept
// - Panes are given IDs upon creation by TerminalTab
// Arguments:
// - The number to set this pane's ID to
void Pane::Id(uint16_t id) noexcept
void Pane::Id(uint32_t id) noexcept
{
_id = id;
}
Expand All @@ -1583,20 +1590,24 @@ void Pane::Id(uint16_t id) noexcept
// - Recursive function that focuses a pane with the given ID
// Arguments:
// - The ID of the pane we want to focus
void Pane::FocusPane(const uint16_t id)
bool Pane::FocusPane(const uint32_t id)
{
if (_IsLeaf() && id == _id)
{
_control.Focus(FocusState::Programmatic);
// Make sure to use _FocusFirstChild here - that'll properly update the
// focus if we're in startup.
_FocusFirstChild();
return true;
Copy link
Member

Choose a reason for hiding this comment

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

If this returns, you don't need the else.

}
else
{
if (_firstChild && _secondChild)
{
_firstChild->FocusPane(id);
_secondChild->FocusPane(id);
return _firstChild->FocusPane(id) ||
_secondChild->FocusPane(id);
}
}
return false;
}

// Method Description:
Expand Down
8 changes: 4 additions & 4 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,9 @@ class Pane : public std::enable_shared_from_this<Pane>
void Maximize(std::shared_ptr<Pane> zoomedPane);
void Restore(std::shared_ptr<Pane> zoomedPane);

std::optional<uint16_t> Id() noexcept;
void Id(uint16_t id) noexcept;
void FocusPane(const uint16_t id);
std::optional<uint32_t> Id() noexcept;
void Id(uint32_t id) noexcept;
bool FocusPane(const uint32_t id);

bool ContainsReadOnly() const;

Expand All @@ -103,7 +103,7 @@ class Pane : public std::enable_shared_from_this<Pane>
winrt::Microsoft::Terminal::Settings::Model::SplitState _splitState{ winrt::Microsoft::Terminal::Settings::Model::SplitState::None };
float _desiredSplitPosition;

std::optional<uint16_t> _id;
std::optional<uint32_t> _id;

bool _lastActive{ false };
std::optional<GUID> _profile{ std::nullopt };
Expand Down
Loading