Skip to content

Commit

Permalink
Add support for focusPane action, focus-pane subcommand (#10142)
Browse files Browse the repository at this point in the history
## Summary of the Pull Request

Adds support for the `focusPane` action, and the `focus-pane` subcommand. These allow the user to focus a pane by it's ID. 

* `focusPane` accepts an `id`, identifying the id of the pane to focus.
* `focus-pane`, `fp` requires the parameter `--target,-t` to ID the pane it's going to focus.

## PR Checklist
* [x] Closes #5803
* [x] Closes #5464
* [x] I work here
* [x] Tests added/passed
* [ ] Requires documentation to be updated - oh no

## Detailed Description of the Pull Request / Additional comments

The ID isn't _totally_ useful right now, since users can't see them. But they're there, and used in-order. This is just slightly more ergonomic for complicated commandlines than `mf up; mf left`

## Validation Steps Performed

Tested in command palette
Tested a variety of commandlines. `wtd -w 0 mf down ; sp` and `wtd -w 0 fp -t 1 ; sp` gave me special difficulty.
  • Loading branch information
zadjii-msft committed May 21, 2021
1 parent d6288fa commit 3f82613
Show file tree
Hide file tree
Showing 16 changed files with 342 additions and 54 deletions.
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));
}
}
}
}
}
47 changes: 45 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,45 @@ 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.
if (_focusPaneTarget >= 0)
{
ActionAndArgs focusPaneAction{};
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 +574,8 @@ bool AppCommandlineArgs::_noCommandsProvided()
*_focusTabShort ||
*_moveFocusCommand ||
*_moveFocusShort ||
*_focusPaneCommand ||
*_focusPaneShort ||
*_newPaneShort.subcommand ||
*_newPaneCommand.subcommand);
}
Expand Down Expand Up @@ -567,6 +607,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;
}
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

0 comments on commit 3f82613

Please sign in to comment.