From 7235996b4d83b340a9cfa7227381922e5ad7baf8 Mon Sep 17 00:00:00 2001 From: Mike Griese Date: Mon, 11 Jan 2021 12:37:05 -0600 Subject: [PATCH] Add a `move-focus` subcommand (#8546) ## Summary of the Pull Request Adds support for the `move-focus` subcommand to `wt.exe`. This subcommand works _exactly_ like `moveFocus(up|down|left|right)`. ## References * Will surely conflict with #8183 * Is goodness even in the world where #5464 exists ## PR Checklist * [x] Closes #6580 * [x] I work here * [x] Tests added/passed * [x] Docs PR: MicrosoftDocs/terminal#209 ## Detailed Description of the Pull Request / Additional comments Bear with me, I wrote this before paternity leave, so code might be a bit stale. Oddly, after startup, this _does not_ leave the focus on the pane you moved to. If you `move-focus` during startup, at the end of startup, we'll still focus a _random_ pane. This is because the terminal still auto-focus a TermControl when it's done with layout. While we'll maintain the active control just fine during the startup, at the end of startup, all the controls will complete layout in a random order. This is no different than the startup right now. `wt sp ; sp ; sp` will focus a random pane at the end. This is left for a future someone to fix This is also subject to #2398 / #4692. Moving in a direction isn't _totally_ reliable currently. `focus-pane -t ID` will certainly be more reliable, but this will work in the meantime? ## Validation Steps Performed Opened probably 100 terminals, confirmed that the layout was always correct. Final focused pane was random, but the layout was right. --- .../CommandlineTest.cpp | 136 ++++++++++++++++++ .../TerminalApp/AppCommandlineArgs.cpp | 52 +++++++ src/cascadia/TerminalApp/AppCommandlineArgs.h | 6 + src/cascadia/TerminalApp/Pane.cpp | 13 ++ .../Resources/en-US/Resources.resw | 10 ++ 5 files changed, 217 insertions(+) diff --git a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp index b9261c1c683..10a06284536 100644 --- a/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp +++ b/src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp @@ -47,6 +47,7 @@ namespace TerminalAppLocalTests TEST_METHOD(ParseSplitPaneIntoArgs); TEST_METHOD(ParseComboCommandlineIntoArgs); TEST_METHOD(ParseFocusTabArgs); + TEST_METHOD(ParseMoveFocusArgs); TEST_METHOD(ParseArgumentsWithParsingTerminators); TEST_METHOD(ParseNoCommandIsNewTab); @@ -76,6 +77,23 @@ namespace TerminalAppLocalTests appArgs.ValidateStartupCommands(); } + void _buildCommandlinesExpectFailureHelper(AppCommandlineArgs& appArgs, + const size_t expectedSubcommands, + std::vector& rawCommands) + { + auto commandlines = AppCommandlineArgs::BuildCommands(rawCommands); + VERIFY_ARE_EQUAL(expectedSubcommands, commandlines.size()); + for (auto& cmdBlob : commandlines) + { + const auto result = appArgs.ParseCommand(cmdBlob); + VERIFY_ARE_NOT_EQUAL(0, result); + VERIFY_ARE_NOT_EQUAL("", appArgs._exitMessage); + Log::Comment(NoThrowString().Format( + L"Exit Message:\n%hs", + appArgs._exitMessage.c_str())); + } + } + void _logCommandline(std::vector& rawCommands) { std::wstring buffer; @@ -995,6 +1013,124 @@ namespace TerminalAppLocalTests } } + void CommandlineTest::ParseMoveFocusArgs() + { + 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 `mf` instead of `move-focus`"); + const wchar_t* subcommand = useShortForm ? L"mf" : L"move-focus"; + + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand }; + Log::Comment(NoThrowString().Format( + L"Just the subcommand, without a direction, should fail.")); + + _buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands); + } + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand, L"left" }; + _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::MoveFocus, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + auto myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.FocusDirection()); + } + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand, L"right" }; + _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::MoveFocus, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + auto myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_ARE_EQUAL(FocusDirection::Right, myArgs.FocusDirection()); + } + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand, L"up" }; + _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::MoveFocus, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + auto myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_ARE_EQUAL(FocusDirection::Up, myArgs.FocusDirection()); + } + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand, L"down" }; + _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::MoveFocus, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + auto myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_ARE_EQUAL(FocusDirection::Down, myArgs.FocusDirection()); + } + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand, L"badDirection" }; + Log::Comment(NoThrowString().Format( + L"move-focus with an invalid direction should fail.")); + _buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands); + } + { + AppCommandlineArgs appArgs{}; + std::vector rawCommands{ L"wt.exe", subcommand, L"left", L";", subcommand, L"right" }; + _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(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.FocusDirection()); + + actionAndArgs = appArgs._startupActions.at(2); + VERIFY_ARE_EQUAL(ShortcutAction::MoveFocus, actionAndArgs.Action()); + VERIFY_IS_NOT_NULL(actionAndArgs.Args()); + myArgs = actionAndArgs.Args().try_as(); + VERIFY_IS_NOT_NULL(myArgs); + VERIFY_ARE_EQUAL(FocusDirection::Right, myArgs.FocusDirection()); + } + } + void CommandlineTest::ValidateFirstCommandIsNewTab() { AppCommandlineArgs appArgs{}; diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp index 54635e5da8d..1aaa0af1bc4 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp @@ -189,6 +189,7 @@ void AppCommandlineArgs::_buildParser() _buildNewTabParser(); _buildSplitPaneParser(); _buildFocusTabParser(); + _buildMoveFocusParser(); } // Method Description: @@ -341,6 +342,54 @@ void AppCommandlineArgs::_buildFocusTabParser() setupSubcommand(_focusTabShort); } +// Method Description: +// - Adds the `move-focus` subcommand and related options to the commandline parser. +// - Additionally adds the `mf` subcommand, which is just a shortened version of `move-focus` +// Arguments: +// - +// Return Value: +// - +void AppCommandlineArgs::_buildMoveFocusParser() +{ + _moveFocusCommand = _app.add_subcommand("move-focus", RS_A(L"CmdMoveFocusDesc")); + _moveFocusShort = _app.add_subcommand("mf", RS_A(L"CmdMFDesc")); + + auto setupSubcommand = [this](auto* subcommand) { + std::map map = { + { "left", FocusDirection::Left }, + { "right", FocusDirection::Right }, + { "up", FocusDirection::Up }, + { "down", FocusDirection::Down } + }; + + auto* directionOpt = subcommand->add_option("direction", + _moveFocusDirection, + RS_A(L"CmdMoveFocusDirectionArgDesc")); + + directionOpt->transform(CLI::CheckedTransformer(map, CLI::ignore_case)); + directionOpt->required(); + // 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]() { + if (_moveFocusDirection != FocusDirection::None) + { + MoveFocusArgs args{ _moveFocusDirection }; + + ActionAndArgs actionAndArgs{}; + actionAndArgs.Action(ShortcutAction::MoveFocus); + actionAndArgs.Args(args); + + _startupActions.push_back(std::move(actionAndArgs)); + } + }); + }; + + setupSubcommand(_moveFocusCommand); + setupSubcommand(_moveFocusShort); +} + // Method Description: // - Add the `NewTerminalArgs` parameters to the given subcommand. This enables // that subcommand to support all the properties in a NewTerminalArgs. @@ -448,6 +497,8 @@ bool AppCommandlineArgs::_noCommandsProvided() *_newTabShort.subcommand || *_focusTabCommand || *_focusTabShort || + *_moveFocusCommand || + *_moveFocusShort || *_newPaneShort.subcommand || *_newPaneCommand.subcommand); } @@ -475,6 +526,7 @@ void AppCommandlineArgs::_resetStateToDefault() _focusNextTab = false; _focusPrevTab = false; + _moveFocusDirection = FocusDirection::None; // 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). diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.h b/src/cascadia/TerminalApp/AppCommandlineArgs.h index 33c2f621830..0bdf990e751 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.h +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.h @@ -74,6 +74,9 @@ class TerminalApp::AppCommandlineArgs final NewPaneSubcommand _newPaneShort; CLI::App* _focusTabCommand; CLI::App* _focusTabShort; + CLI::App* _moveFocusCommand; + CLI::App* _moveFocusShort; + // Are you adding a new sub-command? Make sure to update _noCommandsProvided! std::string _profileName; @@ -81,6 +84,8 @@ class TerminalApp::AppCommandlineArgs final std::string _startingTitle; std::string _startingTabColor; + winrt::Microsoft::Terminal::Settings::Model::FocusDirection _moveFocusDirection{ winrt::Microsoft::Terminal::Settings::Model::FocusDirection::None }; + // _commandline will contain the command line with which we'll be spawning a new terminal std::vector _commandline; @@ -106,6 +111,7 @@ class TerminalApp::AppCommandlineArgs final void _buildNewTabParser(); void _buildSplitPaneParser(); void _buildFocusTabParser(); + void _buildMoveFocusParser(); bool _noCommandsProvided(); void _resetStateToDefault(); int _handleExit(const CLI::App& command, const CLI::Error& e); diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index a9f1767e13d..914be085e8e 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -583,6 +583,19 @@ 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()); + } + _control.Focus(FocusState::Programmatic); } else diff --git a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw index 3f122114070..fd4d2714424 100644 --- a/src/cascadia/TerminalApp/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalApp/Resources/en-US/Resources.resw @@ -318,6 +318,16 @@ Launch the window in fullscreen mode + + Move focus to the adjacent pane in the specified direction + + + An alias for the "move-focus" subcommand. + {Locked="\"move-focus\""} + + + The direction to move focus in + Launch the window in focus mode