From bfd982dcbe52f536dd35715b96432c660c990fc4 Mon Sep 17 00:00:00 2001 From: Schuyler Rosefield Date: Wed, 11 Aug 2021 21:23:26 -0400 Subject: [PATCH 1/3] GH10909 add in order movement. Refactor FocusPane/SwapPane to have a shared NavigateDirection function. --- doc/cascadia/profiles.schema.json | 8 +- .../TerminalApp/AppActionHandlers.cpp | 4 +- .../TerminalApp/AppCommandlineArgs.cpp | 27 +- src/cascadia/TerminalApp/Pane.cpp | 324 ++++++++++++------ src/cascadia/TerminalApp/Pane.h | 28 +- src/cascadia/TerminalApp/TerminalPage.cpp | 5 +- src/cascadia/TerminalApp/TerminalPage.h | 2 +- src/cascadia/TerminalApp/TerminalTab.cpp | 22 +- src/cascadia/TerminalApp/TerminalTab.h | 2 +- .../TerminalSettingsModel/ActionArgs.cpp | 8 + .../TerminalSettingsModel/ActionArgs.idl | 4 +- .../TerminalSettingsModel/JsonUtils.h | 2 +- .../Resources/en-US/Resources.resw | 14 +- .../TerminalSettingsSerializationHelpers.h | 4 +- .../TerminalSettingsModel/defaults.json | 4 + 15 files changed, 304 insertions(+), 154 deletions(-) diff --git a/doc/cascadia/profiles.schema.json b/doc/cascadia/profiles.schema.json index 2c736dd2f2a..6071d6b019c 100644 --- a/doc/cascadia/profiles.schema.json +++ b/doc/cascadia/profiles.schema.json @@ -286,7 +286,9 @@ "right", "up", "down", - "previous" + "previous", + "nextInOrder", + "previousInOrder" ], "type": "string" }, @@ -527,7 +529,7 @@ "direction": { "$ref": "#/definitions/FocusDirection", "default": "left", - "description": "The direction to move focus in, between panes. Direction can be 'previous' to move to the most recently used pane." + "description": "The direction to move focus in, between panes. Direction can be 'previous' to move to the most recently used pane, or 'nextInOrder' or 'previousInOrder' to move to the next or previous pane." } } } @@ -544,7 +546,7 @@ "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." + "description": "The direction to move the focus pane in, swapping panes. Direction can be 'previous' to swap with the most recently used pane, or 'nextInOrder' or 'previousInOrder' to move to the next or previous pane." } } } diff --git a/src/cascadia/TerminalApp/AppActionHandlers.cpp b/src/cascadia/TerminalApp/AppActionHandlers.cpp index 3960b6c3893..9aabdc24868 100644 --- a/src/cascadia/TerminalApp/AppActionHandlers.cpp +++ b/src/cascadia/TerminalApp/AppActionHandlers.cpp @@ -349,8 +349,8 @@ namespace winrt::TerminalApp::implementation } else { - _SwapPane(realArgs.Direction()); - args.Handled(true); + auto swapped = _SwapPane(realArgs.Direction()); + args.Handled(swapped); } } } diff --git a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp index 5e7581d8fab..d9ac41dc2ca 100644 --- a/src/cascadia/TerminalApp/AppCommandlineArgs.cpp +++ b/src/cascadia/TerminalApp/AppCommandlineArgs.cpp @@ -389,6 +389,15 @@ void AppCommandlineArgs::_buildFocusTabParser() setupSubcommand(_focusTabShort); } +static const std::map focusDirectionMap = { + { "left", FocusDirection::Left }, + { "right", FocusDirection::Right }, + { "up", FocusDirection::Up }, + { "down", FocusDirection::Down }, + { "nextInOrder", FocusDirection::NextInOrder }, + { "previousInOrder", FocusDirection::PreviousInOrder }, +}; + // 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` @@ -402,18 +411,11 @@ void AppCommandlineArgs::_buildMoveFocusParser() _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->transform(CLI::CheckedTransformer(focusDirectionMap, 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 @@ -448,18 +450,11 @@ void AppCommandlineArgs::_buildSwapPaneParser() _swapPaneCommand = _app.add_subcommand("swap-pane", RS_A(L"CmdSwapPaneDesc")); 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", _swapPaneDirection, RS_A(L"CmdSwapPaneDirectionArgDesc")); - directionOpt->transform(CLI::CheckedTransformer(map, CLI::ignore_case)); + directionOpt->transform(CLI::CheckedTransformer(focusDirectionMap, 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 diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index 439f413131b..f78a1c61e3f 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -214,48 +214,172 @@ bool Pane::ResizePane(const ResizeDirection& direction) } // Method Description: -// - Attempts to move focus to one of our children. If we have a focused child, -// we'll try to move the focus in the direction requested. -// - If there isn't a pane that exists as a child of this pane in the correct -// direction, we'll return false. This will indicate to our parent that they -// should try and move the focus themselves. In this way, the focus can move -// up and down the tree to the correct pane. -// - This method is _very_ similar to SwapPane. Both are trying to find the -// right pane to move (focus) in a direction. +// - Attempt to navigate from the sourcePane according to direction. +// - If the direction is NextInOrder or PreviousInOrder, the next or previous +// leaf in the tree, respectively, will be returned. +// - If the direction is {Up, Down, Left, Right} then the visually-adjacent +// neighbor (if it exists) will be returned. If there are multiple options +// then the first-most leaf will be selected. // Arguments: -// - direction: The direction to move the focus in. +// - sourcePane: the pane to navigate from +// - direction: which direction to go in // Return Value: -// - true if we or a child handled this focus move request. -bool Pane::NavigateFocus(const FocusDirection& direction) +// - The result of navigating from source according to direction, which may be +// nullptr (i.e. no pane was found in that direction). +std::shared_ptr Pane::NavigateDirection(const std::shared_ptr sourcePane, const FocusDirection& direction) { - // If we're a leaf, do nothing. We can't possibly have a descendant with a - // separator the correct direction. + // Can't navigate anywhere if we are a leaf if (_IsLeaf()) { - return false; + return nullptr; + } + + // If the MRU previous pane is requested we can't move; the tab handles MRU + if (direction == FocusDirection::None || direction == FocusDirection::Previous) + { + return nullptr; } - // If the focus direction does not match the split direction, the focused pane + // Check if we in-order traversal is requested + if (direction == FocusDirection::NextInOrder) + { + return NextPane(sourcePane); + } + + if (direction == FocusDirection::PreviousInOrder) + { + return PreviousPane(sourcePane); + } + + // We are left with directional traversal now + // If the focus direction does not match the split direction, the source pane // and its neighbor must necessarily be contained within the same child. if (!DirectionMatchesSplit(direction, _splitState)) { - return _firstChild->NavigateFocus(direction) || _secondChild->NavigateFocus(direction); + if (auto p = _firstChild->NavigateDirection(sourcePane, direction)) + { + return p; + } + return _secondChild->NavigateDirection(sourcePane, direction); } // Since the direction is the same as our split, it is possible that we must // move focus from from one child to another child. // We now must keep track of state while we recurse. - auto focusNeighborPair = _FindFocusAndNeighbor(direction, { 0, 0 }); + const auto paneNeighborPair = _FindPaneAndNeighbor(sourcePane, direction, { 0, 0 }); - // Once we have found the focused pane and its neighbor, wherever they may - // be we can update the focus. - if (focusNeighborPair.focus && focusNeighborPair.neighbor) + if (paneNeighborPair.source && paneNeighborPair.neighbor) { - focusNeighborPair.neighbor->_FocusFirstChild(); - return true; + return paneNeighborPair.neighbor; } - return false; + return nullptr; +} + +// Method Description: +// - Attempts to find the succeeding pane of the provided pane. +// - NB: If targetPane is not a leaf, then this will return one of its children. +// Arguments: +// - targetPane: The pane to search for. +// Return Value: +// - The next pane in tree order after the target pane (if found) +std::shared_ptr Pane::NextPane(const std::shared_ptr targetPane) +{ + // if we are a leaf pane there is no next pane. + if (_IsLeaf()) + { + return nullptr; + } + + std::shared_ptr firstLeaf = nullptr; + std::shared_ptr nextPane = nullptr; + bool foundTarget = false; + + auto foundNext = WalkTree([&](auto pane) { + // In case the target pane is the last pane in the tree, keep a reference + // to the first leaf so we can wrap around. + if (firstLeaf == nullptr && pane->_IsLeaf()) + { + firstLeaf = pane; + } + + // If we've found the target pane already, get the next leaf pane. + if (foundTarget && pane->_IsLeaf()) + { + nextPane = pane; + return true; + } + + // Test if we're the target pane so we know to return the next pane. + if (pane == targetPane) + { + foundTarget = true; + } + + return false; + }); + + // If we found the desired pane just return it + if (foundNext) + { + return nextPane; + } + + // If we found the target pane, but not the next pane it means we were the + // last leaf in the tree. + if (foundTarget) + { + return firstLeaf; + } + + return nullptr; +} + +// Method Description: +// - Attempts to find the preceding pane of the provided pane. +// Arguments: +// - targetPane: The pane to search for. +// Return Value: +// - The previous pane in tree order before the target pane (if found) +std::shared_ptr Pane::PreviousPane(const std::shared_ptr targetPane) +{ + // if we are a leaf pane there is no previous pane. + if (_IsLeaf()) + { + return nullptr; + } + + std::shared_ptr lastLeaf = nullptr; + bool foundTarget = false; + + WalkTree([&](auto pane) { + if (pane == targetPane) + { + foundTarget = true; + // If we were not the first leaf, then return the previous leaf. + // Otherwise keep walking the tree to get the last pane. + if (lastLeaf != nullptr) + { + return true; + } + } + + if (pane->_IsLeaf()) + { + lastLeaf = pane; + } + + return false; + }); + + // If we found the target pane then lastLeaf will either be the preceding + // pane or the last pane in the tree if targetPane is the first leaf. + if (foundTarget) + { + return lastLeaf; + } + + return nullptr; } // Method Description: @@ -382,6 +506,10 @@ bool Pane::SwapPanes(std::shared_ptr first, std::shared_ptr second) updateParent(secondParent); } + // For now the first pane is always the focused pane, so re-focus to + // make sure the cursor is still in the terminal since the root was moved. + first->_FocusFirstChild(); + return true; } @@ -453,29 +581,29 @@ bool Pane::_IsAdjacent(const std::shared_ptr first, } // Method Description: -// - Given the focused pane, and its relative position in the tree, attempt to +// - Given the source pane, and its relative position in the tree, attempt to // find its visual neighbor within the current pane's tree. // The neighbor, if it exists, will be a leaf pane. // Arguments: -// - direction: The direction to search in from the focused pane. -// - focus: the focused pane -// - focusIsSecondSide: If the focused pane is on the "second" side (down/right of split) +// - direction: The direction to search in from the source pane. +// - searchResult: the source pane and its relative position. +// - sourceIsSecondSide: If the source pane is on the "second" side (down/right of split) // relative to the branch being searched // - offset: the offset of the current pane // Return Value: // - A tuple of Panes, the first being the focused pane if found, and the second // being the adjacent pane if it exists, and a bool that represents if the move // goes out of bounds. -Pane::FocusNeighborSearch Pane::_FindNeighborForPane(const FocusDirection& direction, - FocusNeighborSearch searchResult, - const bool focusIsSecondSide, - const Pane::PanePoint offset) +Pane::PaneNeighborSearch Pane::_FindNeighborForPane(const FocusDirection& direction, + PaneNeighborSearch searchResult, + const bool sourceIsSecondSide, + const Pane::PanePoint offset) { // Test if the move will go out of boundaries. E.g. if the focus is already // on the second child of some pane and it attempts to move right, there // can't possibly be a neighbor to be found in the first child. - if ((focusIsSecondSide && (direction == FocusDirection::Right || direction == FocusDirection::Down)) || - (!focusIsSecondSide && (direction == FocusDirection::Left || direction == FocusDirection::Up))) + if ((sourceIsSecondSide && (direction == FocusDirection::Right || direction == FocusDirection::Down)) || + (!sourceIsSecondSide && (direction == FocusDirection::Left || direction == FocusDirection::Up))) { return searchResult; } @@ -483,7 +611,7 @@ Pane::FocusNeighborSearch Pane::_FindNeighborForPane(const FocusDirection& direc // If we are a leaf node test if we adjacent to the focus node if (_IsLeaf()) { - if (_IsAdjacent(searchResult.focus, searchResult.focusOffset, shared_from_this(), offset, direction)) + if (_IsAdjacent(searchResult.source, searchResult.sourceOffset, shared_from_this(), offset, direction)) { searchResult.neighbor = shared_from_this(); } @@ -501,30 +629,36 @@ Pane::FocusNeighborSearch Pane::_FindNeighborForPane(const FocusDirection& direc { secondOffset.x += gsl::narrow_cast(_firstChild->GetRootElement().ActualWidth()); } - auto focusNeighborSearch = _firstChild->_FindNeighborForPane(direction, searchResult, focusIsSecondSide, firstOffset); - if (focusNeighborSearch.neighbor) + auto sourceNeighborSearch = _firstChild->_FindNeighborForPane(direction, searchResult, sourceIsSecondSide, firstOffset); + if (sourceNeighborSearch.neighbor) { - return focusNeighborSearch; + return sourceNeighborSearch; } - return _secondChild->_FindNeighborForPane(direction, searchResult, focusIsSecondSide, secondOffset); + return _secondChild->_FindNeighborForPane(direction, searchResult, sourceIsSecondSide, secondOffset); } // Method Description: -// - Searches the tree to find the currently focused pane, and if it exists, the +// - Searches the tree to find the source pane, and if it exists, the // visually adjacent pane by direction. // Arguments: +// - sourcePane: The pane to find the neighbor of. // - direction: The direction to search in from the focused pane. // - offset: The offset, with the top-left corner being (0,0), that the current pane is relative to the root. // Return Value: -// - The (partial) search result. If the search was successful, the focus and its neighbor will be returned. +// - The (partial) search result. If the search was successful, the pane and its neighbor will be returned. // Otherwise, the neighbor will be null and the focus will be null/non-null if it was found. -Pane::FocusNeighborSearch Pane::_FindFocusAndNeighbor(const FocusDirection& direction, const Pane::PanePoint offset) +Pane::PaneNeighborSearch Pane::_FindPaneAndNeighbor(const std::shared_ptr sourcePane, const FocusDirection& direction, const Pane::PanePoint offset) { - // If we are the currently focused pane, return ourselves + // If we are the source pane, return ourselves + if (this == sourcePane.get()) + { + return { shared_from_this(), nullptr, offset }; + } + if (_IsLeaf()) { - return { _lastActive ? shared_from_this() : nullptr, nullptr, offset }; + return { nullptr, nullptr, offset }; } // Search the first child, which has no offset from the parent pane @@ -540,100 +674,47 @@ Pane::FocusNeighborSearch Pane::_FindFocusAndNeighbor(const FocusDirection& dire secondOffset.x += gsl::narrow_cast(_firstChild->GetRootElement().ActualWidth()); } - auto focusNeighborSearch = _firstChild->_FindFocusAndNeighbor(direction, firstOffset); + auto sourceNeighborSearch = _firstChild->_FindPaneAndNeighbor(sourcePane, direction, firstOffset); // If we have both the focus element and its neighbor, we are done - if (focusNeighborSearch.focus && focusNeighborSearch.neighbor) + if (sourceNeighborSearch.source && sourceNeighborSearch.neighbor) { - return focusNeighborSearch; + return sourceNeighborSearch; } // if we only found the focus, then we search the second branch for the // neighbor. - if (focusNeighborSearch.focus) + if (sourceNeighborSearch.source) { // If we can possibly have both sides of a direction, check if the sibling has the neighbor if (DirectionMatchesSplit(direction, _splitState)) { - return _secondChild->_FindNeighborForPane(direction, focusNeighborSearch, false, secondOffset); + return _secondChild->_FindNeighborForPane(direction, sourceNeighborSearch, false, secondOffset); } - return focusNeighborSearch; + return sourceNeighborSearch; } // If we didn't find the focus at all, we need to search the second branch // for the focus (and possibly its neighbor). - focusNeighborSearch = _secondChild->_FindFocusAndNeighbor(direction, secondOffset); + sourceNeighborSearch = _secondChild->_FindPaneAndNeighbor(sourcePane, direction, secondOffset); // We found both so we are done. - if (focusNeighborSearch.focus && focusNeighborSearch.neighbor) + if (sourceNeighborSearch.source && sourceNeighborSearch.neighbor) { - return focusNeighborSearch; + return sourceNeighborSearch; } // We only found the focus, which means that its neighbor might be in the // first branch. - if (focusNeighborSearch.focus) + if (sourceNeighborSearch.source) { // If we can possibly have both sides of a direction, check if the sibling has the neighbor if (DirectionMatchesSplit(direction, _splitState)) { - return _firstChild->_FindNeighborForPane(direction, focusNeighborSearch, true, firstOffset); + return _firstChild->_FindNeighborForPane(direction, sourceNeighborSearch, true, firstOffset); } - return focusNeighborSearch; + return sourceNeighborSearch; } return { nullptr, nullptr, offset }; } -// Method Description: -// - Attempts to swap places of the focused pane with one of our children. This -// will swap with the visually adjacent leaf pane if one exists in the -// direction requested, maintaining the existing tree structure. -// This breaks down into a few possible cases -// - If the move direction would encounter the edge of the pane, no move occurs -// - If the focused pane has a single neighbor according to the direction, -// then it will swap with it. -// - If the focused pane has multiple neighbors, it will swap with the -// first-most leaf of the neighboring panes. -// Arguments: -// - direction: The direction to move the focused pane in. -// Return Value: -// - true if we or a child handled this pane move request. -bool Pane::SwapPane(const FocusDirection& direction) -{ - // If we're a leaf, do nothing. We can't possibly swap anything. - if (_IsLeaf()) - { - return false; - } - - // If we get a request to move to the previous pane return false because - // that needs to be handled at the tab level. - if (direction == FocusDirection::Previous) - { - return false; - } - - // If the move direction does not match the split direction, the focused pane - // and its neighbor must necessarily be contained within the same child. - if (!DirectionMatchesSplit(direction, _splitState)) - { - return _firstChild->SwapPane(direction) || _secondChild->SwapPane(direction); - } - - // Since the direction is the same as our split, it is possible that we must - // swap a pane from one child to the other child. - // We now must keep track of state while we recurse. - auto focusNeighborPair = _FindFocusAndNeighbor(direction, { 0, 0 }); - - // Once we have found the focused pane and its neighbor, wherever they may - // be, we can swap them. - if (focusNeighborPair.focus && focusNeighborPair.neighbor) - { - auto swapped = SwapPanes(focusNeighborPair.focus, focusNeighborPair.neighbor); - focusNeighborPair.focus->_FocusFirstChild(); - return swapped; - } - - return false; -} - // Method Description: // - Called when our attached control is closed. Triggers listeners to our close // event, if we're a leaf pane. @@ -2074,6 +2155,33 @@ bool Pane::FocusPane(const uint32_t id) } return false; } +// Method Description: +// - Focuses the given pane if it is in the tree. +// This deliberately mirrors FocusPane(id) instead of just calling +// _FocusFirstChild directly. +// Arguments: +// - the pane to focus +// Return Value: +// - true if focus was set +bool Pane::FocusPane(const std::shared_ptr pane) +{ + if (_IsLeaf() && this == pane.get()) + { + // Make sure to use _FocusFirstChild here - that'll properly update the + // focus if we're in startup. + _FocusFirstChild(); + return true; + } + else + { + if (_firstChild && _secondChild) + { + return _firstChild->FocusPane(pane) || + _secondChild->FocusPane(pane); + } + } + return false; +} // Method Description: // - Recursive function that finds a pane with the given ID diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index be51dfcad58..510229cbc58 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -67,10 +67,12 @@ class Pane : public std::enable_shared_from_this void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void Relayout(); bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction); - bool NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); - bool SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); + std::shared_ptr NavigateDirection(const std::shared_ptr sourcePane, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); bool SwapPanes(std::shared_ptr first, std::shared_ptr second); + std::shared_ptr NextPane(const std::shared_ptr pane); + std::shared_ptr PreviousPane(const std::shared_ptr pane); + std::pair, std::shared_ptr> Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType, const float splitSize, const GUID& profile, @@ -98,6 +100,7 @@ class Pane : public std::enable_shared_from_this std::optional Id() noexcept; void Id(uint32_t id) noexcept; bool FocusPane(const uint32_t id); + bool FocusPane(const std::shared_ptr pane); std::shared_ptr FindPane(const uint32_t id); bool ContainsReadOnly() const; @@ -137,7 +140,7 @@ class Pane : public std::enable_shared_from_this private: struct PanePoint; - struct FocusNeighborSearch; + struct PaneNeighborSearch; struct SnapSizeResult; struct SnapChildrenSizeResult; struct LayoutSizeNode; @@ -190,12 +193,13 @@ class Pane : public std::enable_shared_from_this std::shared_ptr _FindParentOfPane(const std::shared_ptr pane); bool _IsAdjacent(const std::shared_ptr first, const PanePoint firstOffset, const std::shared_ptr second, const PanePoint secondOffset, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction) const; - FocusNeighborSearch _FindNeighborForPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction, - FocusNeighborSearch searchResult, - const bool focusIsSecondSide, - const PanePoint offset); - FocusNeighborSearch _FindFocusAndNeighbor(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction, - const PanePoint offset); + PaneNeighborSearch _FindNeighborForPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction, + PaneNeighborSearch searchResult, + const bool focusIsSecondSide, + const PanePoint offset); + PaneNeighborSearch _FindPaneAndNeighbor(const std::shared_ptr sourcePane, + const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction, + const PanePoint offset); void _CloseChild(const bool closeFirst); winrt::fire_and_forget _CloseChildRoutine(const bool closeFirst); @@ -264,11 +268,11 @@ class Pane : public std::enable_shared_from_this float y; }; - struct FocusNeighborSearch + struct PaneNeighborSearch { - std::shared_ptr focus; + std::shared_ptr source; std::shared_ptr neighbor; - PanePoint focusOffset; + PanePoint sourceOffset; }; struct SnapSizeResult diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 67f30992cec..48a488d03f9 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1127,13 +1127,14 @@ namespace winrt::TerminalApp::implementation // - direction: The direction to move the focused pane in. // Return Value: // - - void TerminalPage::_SwapPane(const FocusDirection& direction) + bool TerminalPage::_SwapPane(const FocusDirection& direction) { if (const auto terminalTab{ _GetFocusedTabImpl() }) { _UnZoomIfNeeded(); - terminalTab->SwapPane(direction); + return terminalTab->SwapPane(direction); } + return false; } TermControl TerminalPage::_GetActiveControl() diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h index c7996bb1b19..13e4007eeb1 100644 --- a/src/cascadia/TerminalApp/TerminalPage.h +++ b/src/cascadia/TerminalApp/TerminalPage.h @@ -239,7 +239,7 @@ namespace winrt::TerminalApp::implementation void _SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference& customTabSwitcherMode); bool _SelectTab(uint32_t tabIndex); bool _MoveFocus(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); - void _SwapPane(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); + bool _SwapPane(const Microsoft::Terminal::Settings::Model::FocusDirection& direction); bool _MovePane(const uint32_t tabIdx); winrt::Microsoft::Terminal::Control::TermControl _GetActiveControl(); diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 7896cd851c9..289ea04db84 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -664,7 +664,12 @@ namespace winrt::TerminalApp::implementation { // NOTE: This _must_ be called on the root pane, so that it can propagate // throughout the entire tree. - return _rootPane->NavigateFocus(direction); + if (auto newFocus = _rootPane->NavigateDirection(_activePane, direction)) + { + return _rootPane->FocusPane(newFocus); + } + + return false; } } @@ -676,25 +681,32 @@ namespace winrt::TerminalApp::implementation // - direction: The direction to move the pane in. // Return Value: // - - void TerminalTab::SwapPane(const FocusDirection& direction) + bool TerminalTab::SwapPane(const FocusDirection& direction) { if (direction == FocusDirection::Previous) { if (_mruPanes.size() < 2) { - return; + return false; } if (auto lastPane = _rootPane->FindPane(_mruPanes.at(1))) { - _rootPane->SwapPanes(_activePane, lastPane); + return _rootPane->SwapPanes(_activePane, lastPane); } } else { // NOTE: This _must_ be called on the root pane, so that it can propagate // throughout the entire tree. - _rootPane->SwapPane(direction); + if (auto neighbor = _rootPane->NavigateDirection(_activePane, direction)) + { + return _rootPane->SwapPanes(_activePane, neighbor); + } + + return false; } + + return false; } bool TerminalTab::FocusPane(const uint32_t id) diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h index bafa9395191..f22d7e646c6 100644 --- a/src/cascadia/TerminalApp/TerminalTab.h +++ b/src/cascadia/TerminalApp/TerminalTab.h @@ -59,7 +59,7 @@ namespace winrt::TerminalApp::implementation void ResizeContent(const winrt::Windows::Foundation::Size& newSize); void ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction); bool NavigateFocus(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); - void SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); + bool SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction); bool FocusPane(const uint32_t id); void UpdateSettings(const Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings, const GUID& profile); diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp index d422bf2e24a..bd49bf43d0f 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.cpp +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.cpp @@ -292,6 +292,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation break; case FocusDirection::Previous: return RS_(L"MoveFocusToLastUsedPane"); + case FocusDirection::NextInOrder: + return RS_(L"MoveFocusNextInOrder"); + case FocusDirection::PreviousInOrder: + return RS_(L"MoveFocusPreviousInOrder"); } return winrt::hstring{ fmt::format(std::wstring_view(RS_(L"MoveFocusWithArgCommandKey")), @@ -318,6 +322,10 @@ namespace winrt::Microsoft::Terminal::Settings::Model::implementation break; case FocusDirection::Previous: return RS_(L"SwapPaneToLastUsedPane"); + case FocusDirection::NextInOrder: + return RS_(L"SwapPaneNextInOrder"); + case FocusDirection::PreviousInOrder: + return RS_(L"SwapPanePreviousInOrder"); } return winrt::hstring{ fmt::format(std::wstring_view(RS_(L"SwapPaneWithArgCommandKey")), diff --git a/src/cascadia/TerminalSettingsModel/ActionArgs.idl b/src/cascadia/TerminalSettingsModel/ActionArgs.idl index 25e765f6447..ce08759f35d 100644 --- a/src/cascadia/TerminalSettingsModel/ActionArgs.idl +++ b/src/cascadia/TerminalSettingsModel/ActionArgs.idl @@ -33,7 +33,9 @@ namespace Microsoft.Terminal.Settings.Model Right, Up, Down, - Previous + Previous, + PreviousInOrder, + NextInOrder }; enum SplitState diff --git a/src/cascadia/TerminalSettingsModel/JsonUtils.h b/src/cascadia/TerminalSettingsModel/JsonUtils.h index fa8dbcb499b..220ec1df372 100644 --- a/src/cascadia/TerminalSettingsModel/JsonUtils.h +++ b/src/cascadia/TerminalSettingsModel/JsonUtils.h @@ -101,7 +101,7 @@ namespace Microsoft::Terminal::Settings::Model::JsonUtils { public: DeserializationError(const Json::Value& value) : - runtime_error("failed to deserialize"), + runtime_error(std::string("failed to deserialize ") + (value.isNull() ? "" : value.asCString())), jsonValue{ value } {} void SetKey(std::string_view newKey) diff --git a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw index 4ea614c2577..61038b72b7b 100644 --- a/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw @@ -436,4 +436,16 @@ Windows Console Host Name describing the usage of the classic windows console as the terminal UI. (`conhost.exe`) - + + Move focus to the next pane in order + + + Move focus to the previous pane in order + + + Swap panes with the next pane in order + + + Swap panes with the previous pane in order + + \ No newline at end of file diff --git a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h index bed14cc8124..8d3961dcb72 100644 --- a/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h +++ b/src/cascadia/TerminalSettingsModel/TerminalSettingsSerializationHelpers.h @@ -337,12 +337,14 @@ struct ::Microsoft::Terminal::Settings::Model::JsonUtils::ConversionTrait<::winr // Possible FocusDirection values JSON_ENUM_MAPPER(::winrt::Microsoft::Terminal::Settings::Model::FocusDirection) { - JSON_MAPPINGS(5) = { + JSON_MAPPINGS(7) = { pair_type{ "left", ValueType::Left }, pair_type{ "right", ValueType::Right }, pair_type{ "up", ValueType::Up }, pair_type{ "down", ValueType::Down }, pair_type{ "previous", ValueType::Previous }, + pair_type{ "previousInOrder", ValueType::PreviousInOrder }, + pair_type{ "nextInOrder", ValueType::NextInOrder }, }; }; diff --git a/src/cascadia/TerminalSettingsModel/defaults.json b/src/cascadia/TerminalSettingsModel/defaults.json index 8cfb25cd36f..d5496658248 100644 --- a/src/cascadia/TerminalSettingsModel/defaults.json +++ b/src/cascadia/TerminalSettingsModel/defaults.json @@ -344,11 +344,15 @@ { "command": { "action": "moveFocus", "direction": "right" }, "keys": "alt+right" }, { "command": { "action": "moveFocus", "direction": "up" }, "keys": "alt+up" }, { "command": { "action": "moveFocus", "direction": "previous" }, "keys": "ctrl+alt+left"}, + { "command": { "action": "moveFocus", "direction": "previousInOrder" } }, + { "command": { "action": "moveFocus", "direction": "nextInOrder" } }, { "command": { "action": "swapPane", "direction": "down" } }, { "command": { "action": "swapPane", "direction": "left" } }, { "command": { "action": "swapPane", "direction": "right" } }, { "command": { "action": "swapPane", "direction": "up" } }, { "command": { "action": "swapPane", "direction": "previous"} }, + { "command": { "action": "swapPane", "direction": "previousInOrder"} }, + { "command": { "action": "swapPane", "direction": "nextInOrder"} }, { "command": "togglePaneZoom" }, { "command": "toggleSplitOrientation" }, { "command": "toggleReadOnlyMode" }, From 130a4b272540fa6f475286cf551e01d464229f44 Mon Sep 17 00:00:00 2001 From: Schuyler Rosefield Date: Mon, 16 Aug 2021 17:58:38 -0400 Subject: [PATCH 2/3] Fix comments about return values that now return bools. --- src/cascadia/TerminalApp/TerminalPage.cpp | 6 +++++- src/cascadia/TerminalApp/TerminalTab.cpp | 4 ++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 48a488d03f9..73407e56713 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1126,7 +1126,7 @@ namespace winrt::TerminalApp::implementation // Arguments: // - direction: The direction to move the focused pane in. // Return Value: - // - + // - true if panes were swapped. bool TerminalPage::_SwapPane(const FocusDirection& direction) { if (const auto terminalTab{ _GetFocusedTabImpl() }) @@ -1201,8 +1201,12 @@ namespace winrt::TerminalApp::implementation // specified tab. If the tab index is greater than the number of // tabs, then a new tab will be created for the pane. Similarly, if a pane // is the last remaining pane on a tab, that tab will be closed upon moving. + // - No move will occur if the tabIdx is the same as the current tab, or if + // the specified tab is not a host of terminals (such as the settings tab). // Arguments: // - tabIdx: The target tab index. + // Return Value: + // - true if the pane was successfully moved to the new tab. bool TerminalPage::_MovePane(const uint32_t tabIdx) { auto focusedTab{ _GetFocusedTabImpl() }; diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp index 289ea04db84..2fbca6c5606 100644 --- a/src/cascadia/TerminalApp/TerminalTab.cpp +++ b/src/cascadia/TerminalApp/TerminalTab.cpp @@ -491,7 +491,7 @@ namespace winrt::TerminalApp::implementation // Arguments: // - // Return Value: - // - The removed pane. + // - The removed pane, if the remove succeeded. std::shared_ptr TerminalTab::DetachPane() { // if we only have one pane, remove it entirely @@ -680,7 +680,7 @@ namespace winrt::TerminalApp::implementation // Arguments: // - direction: The direction to move the pane in. // Return Value: - // - + // - true if two panes were swapped. bool TerminalTab::SwapPane(const FocusDirection& direction) { if (direction == FocusDirection::Previous) From d9f8b90f82114fe3315b1d85eff106a86c03433d Mon Sep 17 00:00:00 2001 From: Schuyler Rosefield Date: Mon, 16 Aug 2021 18:03:10 -0400 Subject: [PATCH 3/3] I guess I added a tab somehow? --- src/cascadia/TerminalApp/TerminalPage.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp index 73407e56713..558b71b8748 100644 --- a/src/cascadia/TerminalApp/TerminalPage.cpp +++ b/src/cascadia/TerminalApp/TerminalPage.cpp @@ -1206,7 +1206,7 @@ namespace winrt::TerminalApp::implementation // Arguments: // - tabIdx: The target tab index. // Return Value: - // - true if the pane was successfully moved to the new tab. + // - true if the pane was successfully moved to the new tab. bool TerminalPage::_MovePane(const uint32_t tabIdx) { auto focusedTab{ _GetFocusedTabImpl() };