GH10909 in order movement (#10927)

Adds new in-order traversal for MoveFocus and SwapPane actions.
Refactors the Pane methods to share a `NavigateDirection`
implementation.

Closes #10909

A large amount of the churn here is just renaming some of the things for
directional movement to reflect that it might not always be based on the
focused pane. `NextPane` and `PreviousPane` are the functions that
actually select the next/previous pane respectively and are the core
component of this PR.

VALIDATION
Created multiple panes on a tab, and tried both forward and backwards
movements with move-focus and swap-pane.

doc/cascadia/profiles.schema.json

@@ -297,7 +297,9 @@
         "right",
         "up",
         "down",
-        "previous"
+        "previous",
+        "nextInOrder",
+        "previousInOrder"
       ],
       "type": "string"
     },
@@ -538,7 +540,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."
             }
           }
         }
@@ -555,7 +557,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."
             }
           }
         }

src/cascadia/TerminalApp/AppActionHandlers.cpp

@@ -349,8 +349,8 @@ namespace winrt::TerminalApp::implementation
             }
             else
             {
-                _SwapPane(realArgs.Direction());
+                auto swapped = _SwapPane(realArgs.Direction());
-                args.Handled(true);
+                args.Handled(swapped);
             }
         }
     }

src/cascadia/TerminalApp/AppCommandlineArgs.cpp

@@ -389,6 +389,15 @@ void AppCommandlineArgs::_buildFocusTabParser()
     setupSubcommand(_focusTabShort);
 }
 
+static const std::map<std::string, FocusDirection> 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<std::string, FocusDirection> 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<std::string, FocusDirection> 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

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,
+// - Attempt to navigate from the sourcePane according to direction.
-//   we'll try to move the focus in the direction requested.
+//   - If the direction is NextInOrder or PreviousInOrder, the next or previous
-//   - If there isn't a pane that exists as a child of this pane in the correct
+//     leaf in the tree, respectively, will be returned.
-//     direction, we'll return false. This will indicate to our parent that they
+//   - If the direction is {Up, Down, Left, Right} then the visually-adjacent
-//     should try and move the focus themselves. In this way, the focus can move
+//     neighbor (if it exists) will be returned. If there are multiple options
-//     up and down the tree to the correct pane.
+//     then the first-most leaf will be selected.
-// - This method is _very_ similar to SwapPane. Both are trying to find the
-//   right pane to move (focus) in a direction.
 // 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.
+// - The result of navigating from source according to direction, which may be
-bool Pane::NavigateFocus(const FocusDirection& direction)
+//   nullptr (i.e. no pane was found in that direction).
+std::shared_ptr<Pane> Pane::NavigateDirection(const std::shared_ptr<Pane> sourcePane, const FocusDirection& direction)
 {
-    // If we're a leaf, do nothing. We can't possibly have a descendant with a
+    // Can't navigate anywhere if we are a leaf
-    // separator the correct direction.
     if (_IsLeaf())
     {
-        return false;
+        return nullptr;
     }
 
-    // If the focus direction does not match the split direction, the focused pane
+    // If the MRU previous pane is requested we can't move; the tab handles MRU
+    if (direction == FocusDirection::None || direction == FocusDirection::Previous)
+    {
+        return nullptr;
+    }
+
+    // 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
+    if (paneNeighborPair.source && paneNeighborPair.neighbor)
-    // be we can update the focus.
-    if (focusNeighborPair.focus && focusNeighborPair.neighbor)
     {
-        focusNeighborPair.neighbor->_FocusFirstChild();
+        return paneNeighborPair.neighbor;
-        return true;
     }
 
-    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> Pane::NextPane(const std::shared_ptr<Pane> targetPane)
+{
+    // if we are a leaf pane there is no next pane.
+    if (_IsLeaf())
+    {
+        return nullptr;
+    }
+
+    std::shared_ptr<Pane> firstLeaf = nullptr;
+    std::shared_ptr<Pane> 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> Pane::PreviousPane(const std::shared_ptr<Pane> targetPane)
+{
+    // if we are a leaf pane there is no previous pane.
+    if (_IsLeaf())
+    {
+        return nullptr;
+    }
+
+    std::shared_ptr<Pane> 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<Pane> first, std::shared_ptr<Pane> 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<Pane> 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.
+// - direction: The direction to search in from the source pane.
-// - focus: the focused pane
+// - searchResult: the source pane and its relative position.
-// - focusIsSecondSide: If the focused pane is on the "second" side (down/right of split)
+// - 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,
+Pane::PaneNeighborSearch Pane::_FindNeighborForPane(const FocusDirection& direction,
-                                                     FocusNeighborSearch searchResult,
+                                                    PaneNeighborSearch searchResult,
-                                                     const bool focusIsSecondSide,
+                                                    const bool sourceIsSecondSide,
-                                                     const Pane::PanePoint offset)
+                                                    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)) ||
+    if ((sourceIsSecondSide && (direction == FocusDirection::Right || direction == FocusDirection::Down)) ||
-        (!focusIsSecondSide && (direction == FocusDirection::Left || direction == FocusDirection::Up)))
+        (!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<float>(_firstChild->GetRootElement().ActualWidth());
     }
-    auto focusNeighborSearch = _firstChild->_FindNeighborForPane(direction, searchResult, focusIsSecondSide, firstOffset);
+    auto sourceNeighborSearch = _firstChild->_FindNeighborForPane(direction, searchResult, sourceIsSecondSide, firstOffset);
-    if (focusNeighborSearch.neighbor)
+    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<Pane> 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<float>(_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> 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

src/cascadia/TerminalApp/Pane.h

@@ -67,10 +67,12 @@ public:
     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);
+    std::shared_ptr<Pane> NavigateDirection(const std::shared_ptr<Pane> sourcePane, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);
-    bool SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);
     bool SwapPanes(std::shared_ptr<Pane> first, std::shared_ptr<Pane> second);
 
+    std::shared_ptr<Pane> NextPane(const std::shared_ptr<Pane> pane);
+    std::shared_ptr<Pane> PreviousPane(const std::shared_ptr<Pane> pane);
+
     std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Split(winrt::Microsoft::Terminal::Settings::Model::SplitState splitType,
                                                                   const float splitSize,
                                                                   const GUID& profile,
@@ -98,6 +100,7 @@ public:
     std::optional<uint32_t> Id() noexcept;
     void Id(uint32_t id) noexcept;
     bool FocusPane(const uint32_t id);
+    bool FocusPane(const std::shared_ptr<Pane> pane);
     std::shared_ptr<Pane> FindPane(const uint32_t id);
 
     bool ContainsReadOnly() const;
@@ -137,7 +140,7 @@ public:
 
 private:
     struct PanePoint;
-    struct FocusNeighborSearch;
+    struct PaneNeighborSearch;
     struct SnapSizeResult;
     struct SnapChildrenSizeResult;
     struct LayoutSizeNode;
@@ -190,12 +193,13 @@ private:
 
     std::shared_ptr<Pane> _FindParentOfPane(const std::shared_ptr<Pane> pane);
     bool _IsAdjacent(const std::shared_ptr<Pane> first, const PanePoint firstOffset, const std::shared_ptr<Pane> second, const PanePoint secondOffset, const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction) const;
-    FocusNeighborSearch _FindNeighborForPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
+    PaneNeighborSearch _FindNeighborForPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
-                                             FocusNeighborSearch searchResult,
+                                            PaneNeighborSearch searchResult,
-                                             const bool focusIsSecondSide,
+                                            const bool focusIsSecondSide,
-                                             const PanePoint offset);
+                                            const PanePoint offset);
-    FocusNeighborSearch _FindFocusAndNeighbor(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
+    PaneNeighborSearch _FindPaneAndNeighbor(const std::shared_ptr<Pane> sourcePane,
-                                              const PanePoint offset);
+                                            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 @@ private:
         float y;
     };
 
-    struct FocusNeighborSearch
+    struct PaneNeighborSearch
     {
-        std::shared_ptr<Pane> focus;
+        std::shared_ptr<Pane> source;
         std::shared_ptr<Pane> neighbor;
-        PanePoint focusOffset;
+        PanePoint sourceOffset;
     };
 
     struct SnapSizeResult

src/cascadia/TerminalApp/TerminalPage.cpp

@@ -1126,14 +1126,15 @@ namespace winrt::TerminalApp::implementation
     // Arguments:
     // - direction: The direction to move the focused pane in.
     // Return Value:
-    // - <none>
+    // - true if panes were swapped.
-    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()
@@ -1200,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() };

src/cascadia/TerminalApp/TerminalPage.h

@@ -239,7 +239,7 @@ namespace winrt::TerminalApp::implementation
         void _SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference<Microsoft::Terminal::Settings::Model::TabSwitcherMode>& 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();

src/cascadia/TerminalApp/TerminalTab.cpp

@@ -491,7 +491,7 @@ namespace winrt::TerminalApp::implementation
     // Arguments:
     // - <none>
     // Return Value:
-    // - The removed pane.
+    // - The removed pane, if the remove succeeded.
     std::shared_ptr<Pane> TerminalTab::DetachPane()
     {
         // if we only have one pane, remove it entirely
@@ -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;
         }
     }
 
@@ -675,26 +680,33 @@ namespace winrt::TerminalApp::implementation
     // Arguments:
     // - direction: The direction to move the pane in.
     // Return Value:
-    // - <none>
+    // - true if two panes were swapped.
-    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)

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);

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")),

src/cascadia/TerminalSettingsModel/ActionArgs.idl

@@ -33,7 +33,9 @@ namespace Microsoft.Terminal.Settings.Model
         Right,
         Up,
         Down,
-        Previous
+        Previous,
+        PreviousInOrder,
+        NextInOrder
     };
 
     enum SplitState

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)

src/cascadia/TerminalSettingsModel/Resources/en-US/Resources.resw

@@ -436,6 +436,18 @@
     <value>Windows Console Host</value>
     <comment>Name describing the usage of the classic windows console as the terminal UI. (`conhost.exe`)</comment>
   </data>
+  <data name="MoveFocusNextInOrder" xml:space="preserve">
+    <value>Move focus to the next pane in order</value>
+  </data>
+  <data name="MoveFocusPreviousInOrder" xml:space="preserve">
+    <value>Move focus to the previous pane in order</value>
+  </data>
+  <data name="SwapPaneNextInOrder" xml:space="preserve">
+    <value>Swap panes with the next pane in order</value>
+  </data>
+  <data name="SwapPanePreviousInOrder" xml:space="preserve">
+    <value>Swap panes with the previous pane in order</value>
+  </data>
   <data name="MinimizeToTrayCommandKey" xml:space="preserve">
     <value>Minimize current window to tray</value>
   </data>

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 },
     };
 };
 

src/cascadia/TerminalSettingsModel/defaults.json

@@ -346,11 +346,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" },