Skip to content

Commit

Permalink
Preliminary work to add Swap Panes functionality (GH Issues 1000, 492…
Browse files Browse the repository at this point in the history 
…2) (#10638)

<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? -->
## Summary of the Pull Request
Add functionality to swap a pane with an adjacent (Up/Down/Left/Right) neighbor.

<!-- Other than the issue solved, is this relevant to any other issues/existing PRs? --> 
## References
This work potentially touches on: #1000 #2398 and #4922
<!-- Please review the items on the PR checklist before submitting-->
## PR Checklist
* [x] Closes a component of #1000 (partially, comment), #4922 (partially, `SwapPanes` function is added but not hooked up, no detach functionality)
* [x] CLA signed. If not, go over [here](https://cla.opensource.microsoft.com/microsoft/Terminal) and sign the CLA
* [x] Tests added/passed
* [ ] Documentation updated. If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
* [x] Schema updated.
* [ ] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

<!-- Provide a more detailed description of the PR, other things fixed or any additional comments/features here -->
## Detailed Description of the Pull Request / Additional comments

Its been a while since I've written C++ code, and it is my first time working on a Windows application. I hope that I have not made too many mistakes.

Work currently done:
- Add boilerplate/infrastructure for argument parsing, hotkeys, event handling
- Adds the `MovePane` function that finds the focused pane, and then tries to find
  a pane that is visually adjacent to according to direction.
- First pass at the `SwapPanes` function that swaps the tree location of two panes
- First working version of helpers `_FindFocusAndNeighbor` and `_FindNeighborFromFocus`
  that search the tree for the currently focused pane, and then climbs back up the tree
  to try to find a sibling pane that is adjacent to it. 
- An `_IsAdjacent' function that tests whether two panes, given their relative offsets, are adjacent to each other according to the direction.

Next steps:
- Once working these functions (`_FindFocusAndNeighbor`, etc) could be utilized to also solve #2398 by updating the `NavigateFocus` function.
- Do we want default hotkeys for the new actions?

<!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well -->
## Validation Steps Performed
At this point, compilation and manual testing of functionality (with hotkeys) by creating panes, adding distinguishers to each pane, and then swapping them around to confirm they went to the right location.
  • Loading branch information
@Rosefield
Rosefield authored Jul 22, 2021
1 parent 41ade2c commit cf97a9f
Show file tree
Hide file tree
Showing 20 changed files with 1,037 additions and 2 deletions.
19 changes: 19 additions & 0 deletions doc/cascadia/profiles.schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@
"identifyWindow",
"identifyWindows",
"moveFocus",
"movePane",
"moveTab",
"newTab",
"newWindow",
Expand Down Expand Up @@ -514,6 +515,23 @@
],
"required": [ "direction" ]
},
"MovePaneAction": {
"description": "Arguments corresponding to a Move Pane Action",
"allOf": [
{ "$ref": "#/definitions/ShortcutAction" },
{
"properties": {
"action": { "type": "string", "pattern": "movePane" },
"direction": {
"$ref": "#/definitions/FocusDirection",
"default": "left",
"description": "The direction to move the focus pane in, swapping panes. Direction can be 'previous' to swap with the most recently used pane."
}
}
}
],
"required": [ "direction" ]
},
"ResizePaneAction": {
"description": "Arguments corresponding to a Resize Pane Action",
"allOf": [
Expand Down Expand Up @@ -952,6 +970,7 @@
{ "$ref": "#/definitions/NewTabAction" },
{ "$ref": "#/definitions/SwitchToTabAction" },
{ "$ref": "#/definitions/MoveFocusAction" },
{ "$ref": "#/definitions/MovePaneAction" },
{ "$ref": "#/definitions/ResizePaneAction" },
{ "$ref": "#/definitions/SendInputAction" },
{ "$ref": "#/definitions/SplitPaneAction" },
Expand Down
119 changes: 119 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/CommandlineTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ namespace TerminalAppLocalTests
TEST_METHOD(ParseComboCommandlineIntoArgs);
TEST_METHOD(ParseFocusTabArgs);
TEST_METHOD(ParseMoveFocusArgs);
TEST_METHOD(ParseMovePaneArgs);
TEST_METHOD(ParseArgumentsWithParsingTerminators);
TEST_METHOD(ParseFocusPaneArgs);

Expand Down Expand Up @@ -1207,6 +1208,124 @@ namespace TerminalAppLocalTests
}
}

void CommandlineTest::ParseMovePaneArgs()
{
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 `mp` instead of `move-pane`");
const wchar_t* subcommand = useShortForm ? L"mp" : L"move-pane";

{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> 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<const wchar_t*> 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::MovePane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.Direction());
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> 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::MovePane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Right, myArgs.Direction());
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> 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::MovePane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Up, myArgs.Direction());
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> 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::MovePane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Down, myArgs.Direction());
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> rawCommands{ L"wt.exe", subcommand, L"badDirection" };
Log::Comment(NoThrowString().Format(
L"move-pane with an invalid direction should fail."));
_buildCommandlinesExpectFailureHelper(appArgs, 1u, rawCommands);
}
{
AppCommandlineArgs appArgs{};
std::vector<const wchar_t*> 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::MovePane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
auto myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Left, myArgs.Direction());

actionAndArgs = appArgs._startupActions.at(2);
VERIFY_ARE_EQUAL(ShortcutAction::MovePane, actionAndArgs.Action());
VERIFY_IS_NOT_NULL(actionAndArgs.Args());
myArgs = actionAndArgs.Args().try_as<MovePaneArgs>();
VERIFY_IS_NOT_NULL(myArgs);
VERIFY_ARE_EQUAL(FocusDirection::Right, myArgs.Direction());
}
}

void CommandlineTest::ParseFocusPaneArgs()
{
BEGIN_TEST_METHOD_PROPERTIES()
Expand Down
208 changes: 208 additions & 0 deletions src/cascadia/LocalTests_TerminalApp/TabTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ namespace TerminalAppLocalTests
TEST_METHOD(MoveFocusFromZoomedPane);
TEST_METHOD(CloseZoomedPane);

TEST_METHOD(MovePanes);

TEST_METHOD(NextMRUTab);
TEST_METHOD(VerifyCommandPaletteTabSwitcherOrder);

Expand Down Expand Up @@ -817,6 +819,212 @@ namespace TerminalAppLocalTests
VERIFY_SUCCEEDED(result);
}

void TabTests::MovePanes()
{
auto page = _commonSetup();

Log::Comment(L"Setup 4 panes.");
// Create the following layout
// -------------------
// | 1 | 2 |
// | | |
// -------------------
// | 3 | 4 |
// | | |
// -------------------
uint32_t firstId = 0, secondId = 0, thirdId = 0, fourthId = 0;
TestOnUIThread([&]() {
VERIFY_ARE_EQUAL(1u, page->_tabs.Size());
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
firstId = tab->_activePane->Id().value();
// We start with 1 tab, split vertically to get
// -------------------
// | 1 | 2 |
// | | |
// -------------------
page->_SplitPane(SplitState::Vertical, SplitType::Duplicate, 0.5f, nullptr);
secondId = tab->_activePane->Id().value();
});
Sleep(250);
TestOnUIThread([&]() {
// After this the `2` pane is focused, go back to `1` being focused
page->_MoveFocus(FocusDirection::Left);
});
Sleep(250);
TestOnUIThread([&]() {
// Split again to make the 3rd tab
// -------------------
// | 1 | |
// | | |
// ---------| 2 |
// | 3 | |
// | | |
// -------------------
page->_SplitPane(SplitState::Horizontal, SplitType::Duplicate, 0.5f, nullptr);
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
// Split again to make the 3rd tab
thirdId = tab->_activePane->Id().value();
});
Sleep(250);
TestOnUIThread([&]() {
// After this the `3` pane is focused, go back to `2` being focused
page->_MoveFocus(FocusDirection::Right);
});
Sleep(250);
TestOnUIThread([&]() {
// Split to create the final pane
// -------------------
// | 1 | 2 |
// | | |
// -------------------
// | 3 | 4 |
// | | |
// -------------------
page->_SplitPane(SplitState::Horizontal, SplitType::Duplicate, 0.5f, nullptr);
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
fourthId = tab->_activePane->Id().value();
});

Sleep(250);
TestOnUIThread([&]() {
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(4, tab->GetLeafPaneCount());
// just to be complete, make sure we actually have 4 different ids
VERIFY_ARE_NOT_EQUAL(firstId, fourthId);
VERIFY_ARE_NOT_EQUAL(secondId, fourthId);
VERIFY_ARE_NOT_EQUAL(thirdId, fourthId);
VERIFY_ARE_NOT_EQUAL(firstId, thirdId);
VERIFY_ARE_NOT_EQUAL(secondId, thirdId);
VERIFY_ARE_NOT_EQUAL(firstId, secondId);
});

// Gratuitous use of sleep to make sure that the UI has updated properly
// after each operation.
Sleep(250);
// Now try to move the pane through the tree
Log::Comment(L"Move pane to the left. This should swap panes 3 and 4");
// -------------------
// | 1 | 2 |
// | | |
// -------------------
// | 4 | 3 |
// | | |
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Left };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
});

Sleep(250);

TestOnUIThread([&]() {
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(4, tab->GetLeafPaneCount());
// Our currently focused pane should be `4`
VERIFY_ARE_EQUAL(fourthId, tab->_activePane->Id().value());

// Inspect the tree to make sure we swapped
VERIFY_ARE_EQUAL(fourthId, tab->_rootPane->_firstChild->_secondChild->Id().value());
VERIFY_ARE_EQUAL(thirdId, tab->_rootPane->_secondChild->_secondChild->Id().value());
});

Sleep(250);

Log::Comment(L"Move pane to up. This should swap panes 1 and 4");
// -------------------
// | 4 | 2 |
// | | |
// -------------------
// | 1 | 3 |
// | | |
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Up };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
});

Sleep(250);

TestOnUIThread([&]() {
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(4, tab->GetLeafPaneCount());
// Our currently focused pane should be `4`
VERIFY_ARE_EQUAL(fourthId, tab->_activePane->Id().value());

// Inspect the tree to make sure we swapped
VERIFY_ARE_EQUAL(fourthId, tab->_rootPane->_firstChild->_firstChild->Id().value());
VERIFY_ARE_EQUAL(firstId, tab->_rootPane->_firstChild->_secondChild->Id().value());
});

Sleep(250);

Log::Comment(L"Move pane to the right. This should swap panes 2 and 4");
// -------------------
// | 2 | 4 |
// | | |
// -------------------
// | 1 | 3 |
// | | |
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Right };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
});

Sleep(250);

TestOnUIThread([&]() {
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(4, tab->GetLeafPaneCount());
// Our currently focused pane should be `4`
VERIFY_ARE_EQUAL(fourthId, tab->_activePane->Id().value());

// Inspect the tree to make sure we swapped
VERIFY_ARE_EQUAL(fourthId, tab->_rootPane->_secondChild->_firstChild->Id().value());
VERIFY_ARE_EQUAL(secondId, tab->_rootPane->_firstChild->_firstChild->Id().value());
});

Sleep(250);

Log::Comment(L"Move pane down. This should swap panes 3 and 4");
// -------------------
// | 2 | 3 |
// | | |
// -------------------
// | 1 | 4 |
// | | |
// -------------------
TestOnUIThread([&]() {
// Set up action
MovePaneArgs args{ FocusDirection::Down };
ActionEventArgs eventArgs{ args };

page->_HandleMovePane(nullptr, eventArgs);
});

Sleep(250);

TestOnUIThread([&]() {
auto tab = page->_GetTerminalTabImpl(page->_tabs.GetAt(0));
VERIFY_ARE_EQUAL(4, tab->GetLeafPaneCount());
// Our currently focused pane should be `4`
VERIFY_ARE_EQUAL(fourthId, tab->_activePane->Id().value());

// Inspect the tree to make sure we swapped
VERIFY_ARE_EQUAL(fourthId, tab->_rootPane->_secondChild->_secondChild->Id().value());
VERIFY_ARE_EQUAL(thirdId, tab->_rootPane->_secondChild->_firstChild->Id().value());
});
}

void TabTests::NextMRUTab()
{
// This is a test for GH#8025 - we want to make sure that we can do both
Expand Down
Loading

0 comments on commit cf97a9f

Please sign in to comment.