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

Move Pane to Tab (GH7075) #10780

Merged
24 commits merged into from
Aug 12, 2021
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
0fb4b38
first very-much-not-working pass. Need to figure out how to manage al…
Rosefield Jul 20, 2021
4adfdee
GH7075 Move Pane to Tab
Rosefield Jul 24, 2021
e33b7aa
Remove control event handlers when pane is detached. Make sure event …
Rosefield Jul 25, 2021
64346fb
Appease the spelling bot
Rosefield Jul 25, 2021
d66d02e
Flesh out comments on new methods. run code formatter
Rosefield Jul 25, 2021
b8e95e3
Fix typos
Rosefield Jul 25, 2021
3bb24b2
Anticipate some code review refactoring. Make sure that we clean up e…
Rosefield Jul 29, 2021
4077160
Fix duplicated comment. Only say we handled an action if we actually …
Rosefield Jul 29, 2021
a7d7aec
box the event token so that we can pass a reference to it to the even…
Rosefield Aug 1, 2021
e7a9d21
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 1, 2021
eea166c
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 2, 2021
abe6a2b
Fix bug where border is lost when child is closed. Make it explicit w…
Rosefield Aug 2, 2021
530f658
spelling
Rosefield Aug 2, 2021
d1ed59c
Add action to keybindings in schema
Rosefield Aug 4, 2021
23b1ccf
Make separate DetachRoot function so that logic can be used separatel…
Rosefield Aug 4, 2021
ae1a84a
fix comment
Rosefield Aug 5, 2021
ef5089f
dont crash if we try to move a pane to a non-terminal tab
Rosefield Aug 6, 2021
2188f6b
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 7, 2021
c6c9cc1
Rename move-pane -> swap-pane, move-pane-to-tab -> move-pane
Rosefield Aug 10, 2021
012747a
Merge remote-tracking branch 'origin/main' into feature/gh7075-move-p…
Rosefield Aug 10, 2021
4d9e571
formatting
Rosefield Aug 10, 2021
e91c69c
update tests to be swap-pane as well
Rosefield Aug 10, 2021
61a3cae
Make things that can be const, const
Rosefield Aug 12, 2021
1e048e2
Fix crash introduced with last merge; dont try to ask the root pane f…
Rosefield Aug 12, 2021
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 .github/actions/spelling/allow/apis.txt
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ localtime
lround
LSHIFT
memicmp
mptt
mov
msappx
MULTIPLEUSE
Expand Down
25 changes: 22 additions & 3 deletions doc/cascadia/profiles.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@
"identifyWindows",
"moveFocus",
"movePane",
"swapPane",
"moveTab",
"newTab",
"newWindow",
Expand Down Expand Up @@ -493,6 +494,23 @@
"type": "integer",
"default": 0,
"description": "Which tab to switch to, with the first being 0"
}
}
}
],
"required": [ "index" ]
},
"MovePaneAction": {
"description": "Arguments corresponding to a Move Pane Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "movePane" },
"index": {
"type": "integer",
"default": 0,
"description": "Which tab to move the pane to, with the first being 0"
}
}
}
Expand All @@ -516,13 +534,13 @@
],
"required": [ "direction" ]
},
"MovePaneAction": {
"description": "Arguments corresponding to a Move Pane Action",
"SwapPaneAction": {
"description": "Arguments corresponding to a Swap Pane Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "movePane" },
"action": { "type": "string", "pattern": "swapPane" },
"direction": {
"$ref": "#/definitions/FocusDirection",
"default": "left",
Expand Down Expand Up @@ -972,6 +990,7 @@
{ "$ref": "#/definitions/SwitchToTabAction" },
{ "$ref": "#/definitions/MoveFocusAction" },
{ "$ref": "#/definitions/MovePaneAction" },
{ "$ref": "#/definitions/SwapPaneAction" },
{ "$ref": "#/definitions/ResizePaneAction" },
{ "$ref": "#/definitions/SendInputAction" },
{ "$ref": "#/definitions/SplitPaneAction" },
Expand Down
20 changes: 17 additions & 3 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,20 @@ namespace winrt::TerminalApp::implementation
}
}

void TerminalPage::_HandleMovePane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (args == nullptr)
{
args.Handled(false);
}
else if (const auto& realArgs = args.ActionArgs().try_as<MovePaneArgs>())
{
auto moved = _MovePane(realArgs.TabIndex());
args.Handled(moved);
}
}

void TerminalPage::_HandleSplitPane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
Expand Down Expand Up @@ -323,10 +337,10 @@ namespace winrt::TerminalApp::implementation
}
}

void TerminalPage::_HandleMovePane(const IInspectable& /*sender*/,
void TerminalPage::_HandleSwapPane(const IInspectable& /*sender*/,
const ActionEventArgs& args)
{
if (const auto& realArgs = args.ActionArgs().try_as<MovePaneArgs>())
if (const auto& realArgs = args.ActionArgs().try_as<SwapPaneArgs>())
{
if (realArgs.Direction() == FocusDirection::None)
{
Expand All @@ -335,7 +349,7 @@ namespace winrt::TerminalApp::implementation
}
else
{
_MovePane(realArgs.Direction());
_SwapPane(realArgs.Direction());
args.Handled(true);
}
}
Expand Down
63 changes: 50 additions & 13 deletions src/cascadia/TerminalApp/AppCommandlineArgs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,7 @@ void AppCommandlineArgs::_buildParser()
_buildFocusTabParser();
_buildMoveFocusParser();
_buildMovePaneParser();
_buildSwapPaneParser();
_buildFocusPaneParser();
}

Expand Down Expand Up @@ -297,6 +298,43 @@ void AppCommandlineArgs::_buildSplitPaneParser()
setupSubcommand(_newPaneCommand);
setupSubcommand(_newPaneShort);
}
// Method Description:
// - Adds the `move-pane` subcommand and related options to the commandline parser.
// - Additionally adds the `mp` subcommand, which is just a shortened version of `move-pane`
// Arguments:
// - <none>
// Return Value:
// - <none>
void AppCommandlineArgs::_buildMovePaneParser()
{
_movePaneCommand = _app.add_subcommand("move-pane", RS_A(L"CmdMovePaneDesc"));
_movePaneShort = _app.add_subcommand("mp", RS_A(L"CmdMPDesc"));

auto setupSubcommand = [this](auto* subcommand) {
subcommand->add_option("-t,--tab",
_movePaneTabIndex,
RS_A(L"CmdMovePaneTabArgDesc"));

// 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 movePaneAction{};

if (_movePaneTabIndex >= 0)
{
movePaneAction.Action(ShortcutAction::MovePane);
MovePaneArgs args{ static_cast<unsigned int>(_movePaneTabIndex) };
movePaneAction.Args(args);
_startupActions.push_back(movePaneAction);
}
});
};
setupSubcommand(_movePaneCommand);
setupSubcommand(_movePaneShort);
}

// Method Description:
// - Adds the `focus-tab` subcommand and related options to the commandline parser.
Expand Down Expand Up @@ -400,16 +438,14 @@ void AppCommandlineArgs::_buildMoveFocusParser()
}

// Method Description:
// - Adds the `move-pane` subcommand and related options to the commandline parser.
// - Additionally adds the `mp` subcommand, which is just a shortened version of `move-pane`
// - Adds the `swap-pane` subcommand and related options to the commandline parser.
// Arguments:
// - <none>
// Return Value:
// - <none>
void AppCommandlineArgs::_buildMovePaneParser()
void AppCommandlineArgs::_buildSwapPaneParser()
{
_movePaneCommand = _app.add_subcommand("move-pane", RS_A(L"CmdMovePaneDesc"));
_movePaneShort = _app.add_subcommand("mp", RS_A(L"CmdMPDesc"));
_swapPaneCommand = _app.add_subcommand("swap-pane", RS_A(L"CmdSwapPaneDesc"));

auto setupSubcommand = [this](auto* subcommand) {
std::map<std::string, FocusDirection> map = {
Expand All @@ -420,8 +456,8 @@ void AppCommandlineArgs::_buildMovePaneParser()
};

auto* directionOpt = subcommand->add_option("direction",
_movePaneDirection,
RS_A(L"CmdMovePaneDirectionArgDesc"));
_swapPaneDirection,
RS_A(L"CmdSwapPaneDirectionArgDesc"));

directionOpt->transform(CLI::CheckedTransformer(map, CLI::ignore_case));
directionOpt->required();
Expand All @@ -430,21 +466,20 @@ void AppCommandlineArgs::_buildMovePaneParser()
// that `this` will still be safe - this function just lets us know this
// command was parsed.
subcommand->callback([&, this]() {
if (_movePaneDirection != FocusDirection::None)
if (_swapPaneDirection != FocusDirection::None)
{
MovePaneArgs args{ _movePaneDirection };
SwapPaneArgs args{ _swapPaneDirection };

ActionAndArgs actionAndArgs{};
actionAndArgs.Action(ShortcutAction::MovePane);
actionAndArgs.Action(ShortcutAction::SwapPane);
actionAndArgs.Args(args);

_startupActions.push_back(std::move(actionAndArgs));
}
});
};

setupSubcommand(_movePaneCommand);
setupSubcommand(_movePaneShort);
setupSubcommand(_swapPaneCommand);
}

// Method Description:
Expand Down Expand Up @@ -625,6 +660,7 @@ bool AppCommandlineArgs::_noCommandsProvided()
*_moveFocusShort ||
*_movePaneCommand ||
*_movePaneShort ||
*_swapPaneCommand ||
*_focusPaneCommand ||
*_focusPaneShort ||
*_newPaneShort.subcommand ||
Expand Down Expand Up @@ -653,12 +689,13 @@ void AppCommandlineArgs::_resetStateToDefault()
_splitPaneSize = 0.5f;
_splitDuplicate = false;

_movePaneTabIndex = -1;
_focusTabIndex = -1;
_focusNextTab = false;
_focusPrevTab = false;

_moveFocusDirection = FocusDirection::None;
_movePaneDirection = FocusDirection::None;
_swapPaneDirection = FocusDirection::None;

_focusPaneTarget = -1;

Expand Down
5 changes: 4 additions & 1 deletion src/cascadia/TerminalApp/AppCommandlineArgs.h
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ class TerminalApp::AppCommandlineArgs final
CLI::App* _moveFocusShort;
CLI::App* _movePaneCommand;
CLI::App* _movePaneShort;
CLI::App* _swapPaneCommand;
CLI::App* _focusPaneCommand;
CLI::App* _focusPaneShort;

Expand All @@ -97,7 +98,7 @@ class TerminalApp::AppCommandlineArgs final
bool _suppressApplicationTitle{ false };

winrt::Microsoft::Terminal::Settings::Model::FocusDirection _moveFocusDirection{ winrt::Microsoft::Terminal::Settings::Model::FocusDirection::None };
winrt::Microsoft::Terminal::Settings::Model::FocusDirection _movePaneDirection{ winrt::Microsoft::Terminal::Settings::Model::FocusDirection::None };
winrt::Microsoft::Terminal::Settings::Model::FocusDirection _swapPaneDirection{ winrt::Microsoft::Terminal::Settings::Model::FocusDirection::None };

// _commandline will contain the command line with which we'll be spawning a new terminal
std::vector<std::string> _commandline;
Expand All @@ -107,6 +108,7 @@ class TerminalApp::AppCommandlineArgs final
bool _splitDuplicate{ false };
float _splitPaneSize{ 0.5f };

int _movePaneTabIndex{ -1 };
int _focusTabIndex{ -1 };
bool _focusNextTab{ false };
bool _focusPrevTab{ false };
Expand All @@ -132,6 +134,7 @@ class TerminalApp::AppCommandlineArgs final
void _buildFocusTabParser();
void _buildMoveFocusParser();
void _buildMovePaneParser();
void _buildSwapPaneParser();
void _buildFocusPaneParser();
bool _noCommandsProvided();
void _resetStateToDefault();
Expand Down
Loading