From cfb41579b9ace8d785d15b5294e3e5c00f4f4fa1 Mon Sep 17 00:00:00 2001
From: Schuyler Rosefield <Rosefield@users.noreply.github.com>
Date: Sat, 18 Sep 2021 13:18:24 -0400
Subject: [PATCH 1/4] Refactor: Make WalkTree iterator more useful and add a
 simple _FindTree based on it.

---
 src/cascadia/TerminalApp/Pane.cpp        | 165 ++++++-----------------
 src/cascadia/TerminalApp/Pane.h          |  33 +++--
 src/cascadia/TerminalApp/TerminalTab.cpp |  22 +--
 3 files changed, 69 insertions(+), 151 deletions(-)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index 82bc861b4f3..817116ca4de 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -491,16 +491,8 @@ std::shared_ptr<Pane> Pane::NavigateDirection(const std::shared_ptr<Pane> source
     // Fixed movement
     if (direction == FocusDirection::First)
     {
-        std::shared_ptr<Pane> firstPane = nullptr;
-        WalkTree([&](auto p) {
-            if (p->_IsLeaf())
-            {
-                firstPane = p;
-                return true;
-            }
-
-            return false;
-        });
+        // Just get the first leaf pane.
+        auto firstPane = _FindPane([](auto p) { return p->_IsLeaf(); });
 
         // Don't need to do any movement if we are the source and target pane.
         if (firstPane == sourcePane)
@@ -657,22 +649,9 @@ std::shared_ptr<Pane> Pane::PreviousPane(const std::shared_ptr<Pane> targetPane)
 // - the parent of `pane` if pane is in this tree.
 std::shared_ptr<Pane> Pane::_FindParentOfPane(const std::shared_ptr<Pane> pane)
 {
-    if (_IsLeaf())
-    {
-        return nullptr;
-    }
-
-    if (_firstChild == pane || _secondChild == pane)
-    {
-        return shared_from_this();
-    }
-
-    if (auto p = _firstChild->_FindParentOfPane(pane))
-    {
-        return p;
-    }
-
-    return _secondChild->_FindParentOfPane(pane);
+    return _FindPane([&](auto p) {
+        return p->_firstChild == pane || p->_secondChild == pane;
+    });
 }
 
 // Method Description:
@@ -800,23 +779,15 @@ bool Pane::SwapPanes(std::shared_ptr<Pane> first, std::shared_ptr<Pane> second)
         }
 
         // Refocus the last pane if there was a pane focused
-        first->WalkTree([](auto p) {
-            if (p->_lastActive)
-            {
-                p->_Focus();
-                return true;
-            }
-            return false;
-        });
+        if (const auto focus = first->GetActivePane())
+        {
+            focus->_Focus();
+        }
 
-        second->WalkTree([](auto p) {
-            if (p->_lastActive)
-            {
-                p->_Focus();
-                return true;
-            }
-            return false;
-        });
+        if (const auto focus = second->GetActivePane())
+        {
+            focus->_Focus();
+        }
 
         return true;
     }
@@ -1222,21 +1193,7 @@ Controls::Grid Pane::GetRootElement()
 //   `_lastActive`, else returns this
 std::shared_ptr<Pane> Pane::GetActivePane()
 {
-    if (_lastActive)
-    {
-        return shared_from_this();
-    }
-    if (_IsLeaf())
-    {
-        return nullptr;
-    }
-
-    auto firstFocused = _firstChild->GetActivePane();
-    if (firstFocused != nullptr)
-    {
-        return firstFocused;
-    }
-    return _secondChild->GetActivePane();
+    return _FindPane([](auto p) { return p->_lastActive; });
 }
 
 // Method Description:
@@ -1482,18 +1439,10 @@ std::shared_ptr<Pane> Pane::AttachPane(std::shared_ptr<Pane> pane, SplitDirectio
 
     // If the new pane has a child that was the focus, re-focus it
     // to steal focus from the currently focused pane.
-    if (pane->_HasFocusedChild())
+    if (const auto focus = pane->GetActivePane())
     {
-        pane->WalkTree([](auto p) {
-            if (p->_lastActive)
-            {
-                p->_Focus();
-                return true;
-            }
-            return false;
-        });
+        focus->_Focus();
     }
-
     return first;
 }
 
@@ -1535,7 +1484,6 @@ std::shared_ptr<Pane> Pane::DetachPane(std::shared_ptr<Pane> pane)
         // Trigger the detached event on each child
         detached->WalkTree([](auto pane) {
             pane->_PaneDetachedHandlers(pane);
-            return false;
         });
 
         return detached;
@@ -1624,7 +1572,6 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching)
                     p->_control.ConnectionStateChanged(p->_connectionStateChangedToken);
                     p->_control.WarningBell(p->_warningBellToken);
                 }
-                return false;
             });
         }
 
@@ -1714,7 +1661,6 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching)
                     p->_control.ConnectionStateChanged(p->_connectionStateChangedToken);
                     p->_control.WarningBell(p->_warningBellToken);
                 }
-                return false;
             });
         }
 
@@ -2641,24 +2587,19 @@ void Pane::Id(uint32_t id) noexcept
 bool Pane::FocusPane(const uint32_t id)
 {
     // Always clear the parent child path if we are focusing a leaf
-    _parentChildPath.reset();
-    if (_IsLeaf() && id == _id)
-    {
-        // 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 WalkTree([=](auto p) {
+        p->_parentChildPath.reset();
+        if (p->_id == id)
         {
-            return _firstChild->FocusPane(id) ||
-                   _secondChild->FocusPane(id);
+            // Make sure to use _FocusFirstChild here - that'll properly update the
+            // focus if we're in startup.
+            p->_FocusFirstChild();
+            return true;
         }
-    }
-    return false;
+        return false;
+    });
 }
+
 // Method Description:
 // - Focuses the given pane if it is in the tree.
 // - This is different than FocusPane(id) in that it allows focusing
@@ -2669,22 +2610,16 @@ bool Pane::FocusPane(const uint32_t id)
 // - true if focus was set
 bool Pane::FocusPane(const std::shared_ptr<Pane> pane)
 {
-    if (this == pane.get())
-    {
-        _Focus();
-        return true;
-    }
-    else
-    {
-        // clear the parent child path if we are not the pane being focused.
-        _parentChildPath.reset();
-        if (_firstChild && _secondChild)
+    return WalkTree([&](auto p) {
+        if (p == pane)
         {
-            return _firstChild->FocusPane(pane) ||
-                   _secondChild->FocusPane(pane);
+            p->_Focus();
+            return true;
         }
-    }
-    return false;
+        // clear the parent child path if we are not the pane being focused.
+        p->_parentChildPath.reset();
+        return false;
+    });
 }
 
 // Method Description:
@@ -2695,17 +2630,14 @@ bool Pane::FocusPane(const std::shared_ptr<Pane> pane)
 // - true if the child was found.
 bool Pane::_HasChild(const std::shared_ptr<Pane> child)
 {
-    if (_IsLeaf())
+    if (child == nullptr)
     {
         return false;
     }
 
-    if (_firstChild == child || _secondChild == child)
-    {
-        return true;
-    }
-
-    return _firstChild->_HasChild(child) || _secondChild->_HasChild(child);
+    return WalkTree([&](auto p) {
+        return p->_firstChild == child || p->_secondChild == child;
+    });
 }
 
 // Method Description:
@@ -2716,26 +2648,7 @@ bool Pane::_HasChild(const std::shared_ptr<Pane> child)
 // - A pointer to the pane with the given ID, if found.
 std::shared_ptr<Pane> Pane::FindPane(const uint32_t id)
 {
-    if (_IsLeaf())
-    {
-        if (id == _id)
-        {
-            return shared_from_this();
-        }
-    }
-    else
-    {
-        if (auto pane = _firstChild->FindPane(id))
-        {
-            return pane;
-        }
-        if (auto pane = _secondChild->FindPane(id))
-        {
-            return pane;
-        }
-    }
-
-    return nullptr;
+    return _FindPane([=](auto p) { return p->_IsLeaf() && p->_id == id; });
 }
 
 // Method Description:
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
index 9892a6e8ae2..22c86bc711d 100644
--- a/src/cascadia/TerminalApp/Pane.h
+++ b/src/cascadia/TerminalApp/Pane.h
@@ -142,14 +142,15 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     // Method Description:
     // - A helper method for ad-hoc recursion on a pane tree. Walks the pane
-    //   tree, calling a predicate on each pane in a depth-first pattern.
-    // - If the predicate returns true, recursion is stopped early.
+    //   tree, calling a function on each pane in a depth-first pattern.
+    // - If that function returns void, then it will be called on every pane.
+    // - Otherwise, iteration will continue until a value with operator bool
+    //   returns true.
     // Arguments:
     // - f: The function to be applied to each pane.
     // Return Value:
-    // - true if the predicate returned true on any pane.
+    // - The value of the function applied on a Pane
     template<typename F>
-    //requires std::predicate<F, std::shared_ptr<Pane>>
     auto WalkTree(F f) -> decltype(f(shared_from_this()))
     {
         using R = std::invoke_result_t<F, std::shared_ptr<Pane>>;
@@ -166,20 +167,36 @@ class Pane : public std::enable_shared_from_this<Pane>
         }
         else
         {
-            if (f(shared_from_this()))
+            if (const auto res = f(shared_from_this()))
             {
-                return true;
+                return res;
             }
 
             if (!_IsLeaf())
             {
-                return _firstChild->WalkTree(f) || _secondChild->WalkTree(f);
+                if (const auto res = _firstChild->WalkTree(f))
+                {
+                    return res;
+                }
+                return _secondChild->WalkTree(f);
             }
 
-            return false;
+            return R{};
         }
     }
 
+    template<typename F>
+    std::shared_ptr<Pane> _FindPane(F f)
+    {
+        return WalkTree([f](auto pane) -> std::shared_ptr<Pane> {
+            if (f(pane))
+            {
+                return pane;
+            }
+            return nullptr;
+        });
+    }
+
     void CollectTaskbarStates(std::vector<winrt::TerminalApp::TaskbarState>& states);
 
     WINRT_CALLBACK(Closed, winrt::Windows::Foundation::EventHandler<winrt::Windows::Foundation::IInspectable>);
diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp
index 8ef1847bab3..5cdc14fae23 100644
--- a/src/cascadia/TerminalApp/TerminalTab.cpp
+++ b/src/cascadia/TerminalApp/TerminalTab.cpp
@@ -56,8 +56,6 @@ namespace winrt::TerminalApp::implementation
             {
                 _activePane = pane;
             }
-
-            return false;
         });
 
         // In case none of the panes were already marked as the focus, just
@@ -217,7 +215,6 @@ namespace winrt::TerminalApp::implementation
             {
                 _AttachEventHandlersToControl(pane->Id().value(), control);
             }
-            return false;
         });
     }
 
@@ -586,7 +583,6 @@ namespace winrt::TerminalApp::implementation
         auto p = _rootPane;
         p->WalkTree([](auto pane) {
             pane->_PaneDetachedHandlers(pane);
-            return false;
         });
 
         // Clean up references and close the tab
@@ -614,13 +610,9 @@ namespace winrt::TerminalApp::implementation
             if (p->_IsLeaf())
             {
                 p->Id(_nextPaneId);
+                _AttachEventHandlersToControl(p->Id().value(), p->_control);
                 _nextPaneId++;
             }
-            if (auto control = p->GetTerminalControl())
-            {
-                _AttachEventHandlersToControl(p->Id().value(), control);
-            }
-            return false;
         });
 
         // pass the old id to the new child
@@ -641,14 +633,10 @@ namespace winrt::TerminalApp::implementation
         _AttachEventHandlersToPane(first);
 
         // Make sure that we have the right pane set as the active pane
-        pane->WalkTree([&](auto p) {
-            if (p->_lastActive)
-            {
-                _UpdateActivePane(p);
-                return true;
-            }
-            return false;
-        });
+        if (const auto focus = pane->GetActivePane())
+        {
+            _UpdateActivePane(focus);
+        }
     }
 
     // Method Description:

From 4974d2a3dfe0559b551c94003e207777b5fdb14a Mon Sep 17 00:00:00 2001
From: Schuyler Rosefield <Rosefield@users.noreply.github.com>
Date: Sun, 19 Sep 2021 14:31:32 -0400
Subject: [PATCH 2/4] Consolidate PreCalculateAutoSplit and
 PreCalculateCanSplit since they basically do the same thing.

---
 src/cascadia/TerminalApp/Pane.cpp          | 91 ++++------------------
 src/cascadia/TerminalApp/Pane.h            | 12 +--
 src/cascadia/TerminalApp/TabManagement.cpp |  1 -
 src/cascadia/TerminalApp/TerminalPage.cpp  | 16 ++--
 src/cascadia/TerminalApp/TerminalTab.cpp   | 29 +++----
 src/cascadia/TerminalApp/TerminalTab.h     |  7 +-
 6 files changed, 41 insertions(+), 115 deletions(-)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index 817116ca4de..46c6ac2d90b 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -2208,7 +2208,7 @@ void Pane::_SetupEntranceAnimation()
 //   tree, we'll reduce the size passed to each subsequent recursive call. The
 //   size passed to this method represents how much space this Pane _will_ have
 //   to use.
-//   * If this pane is a leaf, and it's the pane we're looking for, use the
+//   * If this pane is the pane we're looking for, use the
 //     available space to calculate which direction to split in.
 //   * If this pane is _any other leaf_, then just return nullopt, to indicate
 //     that the `target` Pane is not down this branch.
@@ -2219,30 +2219,34 @@ void Pane::_SetupEntranceAnimation()
 // - splitType: The direction we're attempting to split in.
 // - availableSpace: The theoretical space that's available for this pane to be able to split.
 // Return Value:
-// - nullopt if `target` is not this pane or a child of this pane, otherwise
-//   true iff we could split this pane, given `availableSpace`
-// Note:
-// - This method is highly similar to Pane::PreCalculateAutoSplit
-std::optional<bool> Pane::PreCalculateCanSplit(const std::shared_ptr<Pane> target,
-                                               SplitDirection splitType,
-                                               const float splitSize,
-                                               const winrt::Windows::Foundation::Size availableSpace) const
+// - nullopt if the target is not found, or if there is not enough space to fit.
+//   Otherwise will return the "real split direction" (converting automatic splits),
+//   and will return either the split direction or nullopt.
+std::optional<std::optional<SplitDirection>> Pane::PreCalculateCanSplit(const std::shared_ptr<Pane> target,
+                                                                        SplitDirection splitType,
+                                                                        const float splitSize,
+                                                                        const winrt::Windows::Foundation::Size availableSpace) const
 {
     if (target.get() == this)
     {
         const auto firstPercent = 1.0f - splitSize;
         const auto secondPercent = splitSize;
-        // If this pane is a leaf, and it's the pane we're looking for, use
+        // If this pane is the pane we're looking for, use
         // the available space to calculate which direction to split in.
         const Size minSize = _GetMinSize();
 
+        if (splitType == SplitDirection::Automatic)
+        {
+            splitType = availableSpace.Width > availableSpace.Height ? SplitDirection::Right : SplitDirection::Down;
+        }
+
         if (splitType == SplitDirection::Left || splitType == SplitDirection::Right)
         {
             const auto widthMinusSeparator = availableSpace.Width - CombinedPaneBorderSize;
             const auto newFirstWidth = widthMinusSeparator * firstPercent;
             const auto newSecondWidth = widthMinusSeparator * secondPercent;
 
-            return { newFirstWidth > minSize.Width && newSecondWidth > minSize.Width };
+            return newFirstWidth > minSize.Width && newSecondWidth > minSize.Width ? std::optional{ splitType } : std::nullopt;
         }
 
         else if (splitType == SplitDirection::Up || splitType == SplitDirection::Down)
@@ -2251,7 +2255,8 @@ std::optional<bool> Pane::PreCalculateCanSplit(const std::shared_ptr<Pane> targe
             const auto newFirstHeight = heightMinusSeparator * firstPercent;
             const auto newSecondHeight = heightMinusSeparator * secondPercent;
 
-            return { newFirstHeight > minSize.Height && newSecondHeight > minSize.Height };
+            return newFirstHeight > minSize.Height && newSecondHeight > minSize.Height ? std::optional{ splitType } : std::nullopt;
+            ;
         }
     }
     else if (_IsLeaf())
@@ -3086,68 +3091,6 @@ int Pane::GetLeafPaneCount() const noexcept
     return _IsLeaf() ? 1 : (_firstChild->GetLeafPaneCount() + _secondChild->GetLeafPaneCount());
 }
 
-// Method Description:
-// - This is a helper to determine which direction an "Automatic" split should
-//   happen in for a given pane, but without using the ActualWidth() and
-//   ActualHeight() methods. This is used during the initialization of the
-//   Terminal, when we could be processing many "split-pane" commands _before_
-//   we've ever laid out the Terminal for the first time. When this happens, the
-//   Pane's don't have an actual size yet. However, we'd still like to figure
-//   out how to do an "auto" split when these Panes are all laid out.
-// - This method assumes that the Pane we're attempting to split is `target`,
-//   and this method should be called on the root of a tree of Panes.
-// - We'll walk down the tree attempting to find `target`. As we traverse the
-//   tree, we'll reduce the size passed to each subsequent recursive call. The
-//   size passed to this method represents how much space this Pane _will_ have
-//   to use.
-//   * If this pane is a leaf, and it's the pane we're looking for, use the
-//     available space to calculate which direction to split in.
-//   * If this pane is _any other leaf_, then just return nullopt, to indicate
-//     that the `target` Pane is not down this branch.
-//   * If this pane is a parent, calculate how much space our children will be
-//     able to use, and recurse into them.
-// Arguments:
-// - target: The Pane we're attempting to split.
-// - availableSpace: The theoretical space that's available for this pane to be able to split.
-// Return Value:
-// - nullopt if `target` is not this pane or a child of this pane, otherwise the
-//   SplitDirection that `target` would use for an `Automatic` split given
-//   `availableSpace`
-std::optional<SplitDirection> Pane::PreCalculateAutoSplit(const std::shared_ptr<Pane> target,
-                                                          const winrt::Windows::Foundation::Size availableSpace) const
-{
-    if (target.get() == this)
-    {
-        // If this pane is the pane we are looking for, use the available space
-        // to calculate which direction to split in.
-        return availableSpace.Width > availableSpace.Height ? SplitDirection::Right : SplitDirection::Down;
-    }
-    else if (_IsLeaf())
-    {
-        // If this pane is _any other leaf_, then just return nullopt, to
-        // indicate that the `target` Pane is not down this branch.
-        return std::nullopt;
-    }
-    else
-    {
-        // If this pane is a parent, calculate how much space our children will
-        // be able to use, and recurse into them.
-
-        const bool isVerticalSplit = _splitState == SplitState::Vertical;
-        const float firstWidth = isVerticalSplit ? (availableSpace.Width * _desiredSplitPosition) : availableSpace.Width;
-        const float secondWidth = isVerticalSplit ? (availableSpace.Width - firstWidth) : availableSpace.Width;
-        const float firstHeight = !isVerticalSplit ? (availableSpace.Height * _desiredSplitPosition) : availableSpace.Height;
-        const float secondHeight = !isVerticalSplit ? (availableSpace.Height - firstHeight) : availableSpace.Height;
-
-        const auto firstResult = _firstChild->PreCalculateAutoSplit(target, { firstWidth, firstHeight });
-        return firstResult.has_value() ? firstResult : _secondChild->PreCalculateAutoSplit(target, { secondWidth, secondHeight });
-    }
-
-    // We should not possibly be getting here - both the above branches should
-    // return a value.
-    FAIL_FAST();
-}
-
 // Method Description:
 // - Returns true if the pane or one of its descendants is read-only
 bool Pane::ContainsReadOnly() const
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
index 22c86bc711d..856b06212f4 100644
--- a/src/cascadia/TerminalApp/Pane.h
+++ b/src/cascadia/TerminalApp/Pane.h
@@ -114,12 +114,10 @@ class Pane : public std::enable_shared_from_this<Pane>
                                                                   const winrt::Microsoft::Terminal::Control::TermControl& control);
     bool ToggleSplitOrientation();
     float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const;
-    std::optional<winrt::Microsoft::Terminal::Settings::Model::SplitDirection> PreCalculateAutoSplit(const std::shared_ptr<Pane> target,
-                                                                                                     const winrt::Windows::Foundation::Size parentSize) const;
-    std::optional<bool> PreCalculateCanSplit(const std::shared_ptr<Pane> target,
-                                             winrt::Microsoft::Terminal::Settings::Model::SplitDirection splitType,
-                                             const float splitSize,
-                                             const winrt::Windows::Foundation::Size availableSpace) const;
+    std::optional<std::optional<winrt::Microsoft::Terminal::Settings::Model::SplitDirection>> PreCalculateCanSplit(const std::shared_ptr<Pane> target,
+                                                                                                                   winrt::Microsoft::Terminal::Settings::Model::SplitDirection splitType,
+                                                                                                                   const float splitSize,
+                                                                                                                   const winrt::Windows::Foundation::Size availableSpace) const;
     void Shutdown();
     void Close();
 
@@ -298,8 +296,6 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     SplitState _convertAutomaticOrDirectionalSplitState(const winrt::Microsoft::Terminal::Settings::Model::SplitDirection& splitType) const;
 
-    std::optional<winrt::Microsoft::Terminal::Settings::Model::SplitDirection> _preCalculateAutoSplit(const std::shared_ptr<Pane> target, const winrt::Windows::Foundation::Size parentSize) const;
-
     // Function Description:
     // - Returns true if the given direction can be used with the given split
     //   type.
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
index a339b2bfe42..b54d9a508e1 100644
--- a/src/cascadia/TerminalApp/TabManagement.cpp
+++ b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -757,7 +757,6 @@ namespace winrt::TerminalApp::implementation
                                 control.ToggleReadOnly();
                             }
                         }
-                        return false;
                     });
                 }
 
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 25140362de8..cf98666cb67 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1569,14 +1569,10 @@ namespace winrt::TerminalApp::implementation
             const float contentHeight = ::base::saturated_cast<float>(_tabContent.ActualHeight());
             const winrt::Windows::Foundation::Size availableSpace{ contentWidth, contentHeight };
 
-            auto realSplitType = splitDirection;
-            if (realSplitType == SplitDirection::Automatic)
-            {
-                realSplitType = tab.PreCalculateAutoSplit(availableSpace);
-            }
-
-            const auto canSplit = tab.PreCalculateCanSplit(realSplitType, splitSize, availableSpace);
-            if (!canSplit)
+            // PreCalculateCanSplit will convert automatic splits to the appropriate direction
+            // and if there is space to split will return the split direction to use.
+            const auto realSplitType = tab.PreCalculateCanSplit(splitDirection, splitSize, availableSpace);
+            if (!realSplitType)
             {
                 return;
             }
@@ -1587,8 +1583,7 @@ namespace winrt::TerminalApp::implementation
             _RegisterTerminalEvents(newControl);
 
             _UnZoomIfNeeded();
-
-            tab.SplitPane(realSplitType, splitSize, profile, newControl);
+            tab.SplitPane(realSplitType.value(), splitSize, profile, newControl);
 
             // After GH#6586, the control will no longer focus itself
             // automatically when it's finished being laid out. Manually focus
@@ -2223,7 +2218,6 @@ namespace winrt::TerminalApp::implementation
                             pane->UpdateSettings(pair.second, pair.first);
                         }
                     }
-                    return false;
                 });
 
                 // Update the icon of the tab for the currently focused profile in that tab.
diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp
index 5cdc14fae23..4472e1fee07 100644
--- a/src/cascadia/TerminalApp/TerminalTab.cpp
+++ b/src/cascadia/TerminalApp/TerminalTab.cpp
@@ -1565,25 +1565,20 @@ namespace winrt::TerminalApp::implementation
     }
 
     // Method Description:
-    // - This is a helper to determine which direction an "Automatic" split should
-    //   happen in for the active pane of this tab, but without using the ActualWidth() and
-    //   ActualHeight() methods.
-    // - See Pane::PreCalculateAutoSplit
+    // - Calculate if a split is possible with the given direction and size.
+    // - Converts Automatic splits to an appropriate direction depending on space.
     // Arguments:
-    // - availableSpace: The theoretical space that's available for this Tab's content
-    // Return Value:
-    // - The SplitDirection that we should use for an `Automatic` split given
-    //   `availableSpace`
-    SplitDirection TerminalTab::PreCalculateAutoSplit(winrt::Windows::Foundation::Size availableSpace) const
-    {
-        return _rootPane->PreCalculateAutoSplit(_activePane, availableSpace).value_or(SplitDirection::Right);
-    }
-
-    bool TerminalTab::PreCalculateCanSplit(SplitDirection splitType,
-                                           const float splitSize,
-                                           winrt::Windows::Foundation::Size availableSpace) const
+    // - splitType: what direction to split.
+    // - splitSize: how big the new split should be.
+    // - availableSpace: how much space there is to work with.
+    // Return value:
+    // - This will return nullopt if a split of the given size/direction was not possible,
+    //   or it will return the split direction with automatic converted to a cardinal direction.
+    std::optional<SplitDirection> TerminalTab::PreCalculateCanSplit(SplitDirection splitType,
+                                                                    const float splitSize,
+                                                                    winrt::Windows::Foundation::Size availableSpace) const
     {
-        return _rootPane->PreCalculateCanSplit(_activePane, splitType, splitSize, availableSpace).value_or(false);
+        return _rootPane->PreCalculateCanSplit(_activePane, splitType, splitSize, availableSpace).value_or(std::nullopt);
     }
 
     // Method Description:
diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h
index 4c071c0b0b5..4051bb4fd51 100644
--- a/src/cascadia/TerminalApp/TerminalTab.h
+++ b/src/cascadia/TerminalApp/TerminalTab.h
@@ -51,10 +51,9 @@ namespace winrt::TerminalApp::implementation
         winrt::fire_and_forget ActivateBellIndicatorTimer();
 
         float CalcSnappedDimension(const bool widthOrHeight, const float dimension) const;
-        winrt::Microsoft::Terminal::Settings::Model::SplitDirection PreCalculateAutoSplit(winrt::Windows::Foundation::Size rootSize) const;
-        bool PreCalculateCanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitDirection splitType,
-                                  const float splitSize,
-                                  winrt::Windows::Foundation::Size availableSpace) const;
+        std::optional<winrt::Microsoft::Terminal::Settings::Model::SplitDirection> PreCalculateCanSplit(winrt::Microsoft::Terminal::Settings::Model::SplitDirection splitType,
+                                                                                                        const float splitSize,
+                                                                                                        winrt::Windows::Foundation::Size availableSpace) const;
 
         void ResizeContent(const winrt::Windows::Foundation::Size& newSize);
         void ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);

From e51face5e52589cbf96c8fd04ffcabf12d9c6d0f Mon Sep 17 00:00:00 2001
From: Schuyler Rosefield <Rosefield@users.noreply.github.com>
Date: Sat, 18 Sep 2021 13:20:16 -0400
Subject: [PATCH 3/4] Dead code: pane relayout no longer does anything since it
 uses star sizing instead of actual pixel sizing.

---
 src/cascadia/TerminalApp/Pane.cpp          | 50 +---------------------
 src/cascadia/TerminalApp/Pane.h            |  2 -
 src/cascadia/TerminalApp/TabManagement.cpp | 11 -----
 src/cascadia/TerminalApp/TerminalPage.cpp  | 16 -------
 src/cascadia/TerminalApp/TerminalPage.h    |  2 -
 src/cascadia/TerminalApp/TerminalTab.cpp   | 29 -------------
 src/cascadia/TerminalApp/TerminalTab.h     |  2 -
 7 files changed, 1 insertion(+), 111 deletions(-)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index 46c6ac2d90b..65e5d1ee43e 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -266,54 +266,6 @@ Pane::BuildStartupState Pane::BuildStartupActions(uint32_t currentId, uint32_t n
     return { actions, firstState.firstPane, focusedPaneId, firstState.panesCreated + secondState.panesCreated + 1 };
 }
 
-// Method Description:
-// - Update the size of this pane. Resizes each of our columns so they have the
-//   same relative sizes, given the newSize.
-// - Because we're just manually setting the row/column sizes in pixels, we have
-//   to be told our new size, we can't just use our own OnSized event, because
-//   that _won't fire when we get smaller_.
-// Arguments:
-// - newSize: the amount of space that this pane has to fill now.
-// Return Value:
-// - <none>
-void Pane::ResizeContent(const Size& newSize)
-{
-    const auto width = newSize.Width;
-    const auto height = newSize.Height;
-
-    _CreateRowColDefinitions();
-
-    if (_splitState == SplitState::Vertical)
-    {
-        const auto paneSizes = _CalcChildrenSizes(width);
-
-        const Size firstSize{ paneSizes.first, height };
-        const Size secondSize{ paneSizes.second, height };
-        _firstChild->ResizeContent(firstSize);
-        _secondChild->ResizeContent(secondSize);
-    }
-    else if (_splitState == SplitState::Horizontal)
-    {
-        const auto paneSizes = _CalcChildrenSizes(height);
-
-        const Size firstSize{ width, paneSizes.first };
-        const Size secondSize{ width, paneSizes.second };
-        _firstChild->ResizeContent(firstSize);
-        _secondChild->ResizeContent(secondSize);
-    }
-}
-
-// Method Description:
-// - Recalculates and reapplies sizes of all descendant panes.
-// Arguments:
-// - <none>
-// Return Value:
-// - <none>
-void Pane::Relayout()
-{
-    ResizeContent(_root.ActualSize());
-}
-
 // Method Description:
 // - Adjust our child percentages to increase the size of one of our children
 //   and decrease the size of the other.
@@ -350,7 +302,7 @@ bool Pane::_Resize(const ResizeDirection& direction)
     _desiredSplitPosition = _ClampSplitPosition(changeWidth, _desiredSplitPosition - amount, actualDimension);
 
     // Resize our columns to match the new percentages.
-    ResizeContent(actualSize);
+    _CreateRowColDefinitions();
 
     return true;
 }
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
index 856b06212f4..1e875a98420 100644
--- a/src/cascadia/TerminalApp/Pane.h
+++ b/src/cascadia/TerminalApp/Pane.h
@@ -97,8 +97,6 @@ class Pane : public std::enable_shared_from_this<Pane>
 
     void UpdateSettings(const winrt::Microsoft::Terminal::Settings::Model::TerminalSettingsCreateResult& settings,
                         const winrt::Microsoft::Terminal::Settings::Model::Profile& profile);
-    void ResizeContent(const winrt::Windows::Foundation::Size& newSize);
-    void Relayout();
     bool ResizePane(const winrt::Microsoft::Terminal::Settings::Model::ResizeDirection& direction);
     std::shared_ptr<Pane> NavigateDirection(const std::shared_ptr<Pane> sourcePane,
                                             const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction,
diff --git a/src/cascadia/TerminalApp/TabManagement.cpp b/src/cascadia/TerminalApp/TabManagement.cpp
index b54d9a508e1..b3fbb9db8a1 100644
--- a/src/cascadia/TerminalApp/TabManagement.cpp
+++ b/src/cascadia/TerminalApp/TabManagement.cpp
@@ -1036,15 +1036,4 @@ namespace winrt::TerminalApp::implementation
         std::copy(begin(_tabs), end(_tabs), std::back_inserter(tabsToRemove));
         _RemoveTabs(tabsToRemove);
     }
-
-    void TerminalPage::_ResizeTabContent(const winrt::Windows::Foundation::Size& newSize)
-    {
-        for (auto tab : _tabs)
-        {
-            if (auto terminalTab = _GetTerminalTabImpl(tab))
-            {
-                terminalTab->ResizeContent(newSize);
-            }
-        }
-    }
 }
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index cf98666cb67..5c2f65be510 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -241,8 +241,6 @@ namespace winrt::TerminalApp::implementation
 
         _UpdateTabWidthMode();
 
-        _tabContent.SizeChanged({ this, &TerminalPage::_OnContentSizeChanged });
-
         // When the visibility of the command palette changes to "collapsed",
         // the palette has been closed. Toss focus back to the currently active
         // control.
@@ -2119,20 +2117,6 @@ namespace winrt::TerminalApp::implementation
         }
     }
 
-    // Method Description:
-    // - Called when our tab content size changes. This updates each tab with
-    //   the new size, so they have a chance to update each of their panes with
-    //   the new size.
-    // Arguments:
-    // - e: the SizeChangedEventArgs with the new size of the tab content area.
-    // Return Value:
-    // - <none>
-    void TerminalPage::_OnContentSizeChanged(const IInspectable& /*sender*/, Windows::UI::Xaml::SizeChangedEventArgs const& e)
-    {
-        const auto newSize = e.NewSize();
-        _ResizeTabContent(newSize);
-    }
-
     // Method Description:
     // - Responds to the TabView control's Tab Closing event by removing
     //      the indicated tab from the set and focusing another one.
diff --git a/src/cascadia/TerminalApp/TerminalPage.h b/src/cascadia/TerminalApp/TerminalPage.h
index 26332453a66..d82d1a2efb9 100644
--- a/src/cascadia/TerminalApp/TerminalPage.h
+++ b/src/cascadia/TerminalApp/TerminalPage.h
@@ -254,7 +254,6 @@ namespace winrt::TerminalApp::implementation
         void _FocusCurrentTab(const bool focusAlways);
         bool _HasMultipleTabs() const;
         void _RemoveAllTabs();
-        void _ResizeTabContent(const winrt::Windows::Foundation::Size& newSize);
 
         void _SelectNextTab(const bool bMoveRight, const Windows::Foundation::IReference<Microsoft::Terminal::Settings::Model::TabSwitcherMode>& customTabSwitcherMode);
         bool _SelectTab(uint32_t tabIndex);
@@ -336,7 +335,6 @@ namespace winrt::TerminalApp::implementation
         void _OnTabClick(const IInspectable& sender, const Windows::UI::Xaml::Input::PointerRoutedEventArgs& eventArgs);
         void _OnTabSelectionChanged(const IInspectable& sender, const Windows::UI::Xaml::Controls::SelectionChangedEventArgs& eventArgs);
         void _OnTabItemsChanged(const IInspectable& sender, const Windows::Foundation::Collections::IVectorChangedEventArgs& eventArgs);
-        void _OnContentSizeChanged(const IInspectable& /*sender*/, Windows::UI::Xaml::SizeChangedEventArgs const& e);
         void _OnTabCloseRequested(const IInspectable& sender, const Microsoft::UI::Xaml::Controls::TabViewTabCloseRequestedEventArgs& eventArgs);
         void _OnFirstLayout(const IInspectable& sender, const IInspectable& eventArgs);
         void _UpdatedSelectedTab(const winrt::TerminalApp::TabBase& tab);
diff --git a/src/cascadia/TerminalApp/TerminalTab.cpp b/src/cascadia/TerminalApp/TerminalTab.cpp
index 4472e1fee07..c48ce305f29 100644
--- a/src/cascadia/TerminalApp/TerminalTab.cpp
+++ b/src/cascadia/TerminalApp/TerminalTab.cpp
@@ -656,20 +656,6 @@ namespace winrt::TerminalApp::implementation
         return _rootPane->CalcSnappedDimension(widthOrHeight, dimension);
     }
 
-    // Method Description:
-    // - Update the size of our panes to fill the new given size. This happens when
-    //   the window is resized.
-    // Arguments:
-    // - newSize: the amount of space that the panes have to fill now.
-    // Return Value:
-    // - <none>
-    void TerminalTab::ResizeContent(const winrt::Windows::Foundation::Size& newSize)
-    {
-        // NOTE: This _must_ be called on the root pane, so that it can propagate
-        // throughout the entire tree.
-        _rootPane->ResizeContent(newSize);
-    }
-
     // Method Description:
     // - Attempt to move a separator between panes, as to resize each child on
     //   either size of the separator. See Pane::ResizePane for details.
@@ -823,7 +809,6 @@ namespace winrt::TerminalApp::implementation
             auto& events = it->second;
 
             control.TitleChanged(events.titleToken);
-            control.FontSizeChanged(events.fontToken);
             control.TabColorChanged(events.colorToken);
             control.SetTaskbarProgress(events.taskbarToken);
             control.ReadOnlyChanged(events.readOnlyToken);
@@ -860,20 +845,6 @@ namespace winrt::TerminalApp::implementation
             }
         });
 
-        // This is called when the terminal changes its font size or sets it for the first
-        // time (because when we just create terminal via its ctor it has invalid font size).
-        // On the latter event, we tell the root pane to resize itself so that its descendants
-        // (including ourself) can properly snap to character grids. In future, we may also
-        // want to do that on regular font changes.
-        events.fontToken = control.FontSizeChanged([this](const int /* fontWidth */,
-                                                          const int /* fontHeight */,
-                                                          const bool isInitialChange) {
-            if (isInitialChange)
-            {
-                _rootPane->Relayout();
-            }
-        });
-
         events.colorToken = control.TabColorChanged([weakThis](auto&&, auto&&) {
             if (auto tab{ weakThis.get() })
             {
diff --git a/src/cascadia/TerminalApp/TerminalTab.h b/src/cascadia/TerminalApp/TerminalTab.h
index 4051bb4fd51..47fb7f69e33 100644
--- a/src/cascadia/TerminalApp/TerminalTab.h
+++ b/src/cascadia/TerminalApp/TerminalTab.h
@@ -55,7 +55,6 @@ namespace winrt::TerminalApp::implementation
                                                                                                         const float splitSize,
                                                                                                         winrt::Windows::Foundation::Size availableSpace) const;
 
-        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);
         bool SwapPane(const winrt::Microsoft::Terminal::Settings::Model::FocusDirection& direction);
@@ -123,7 +122,6 @@ namespace winrt::TerminalApp::implementation
         struct ControlEventTokens
         {
             winrt::event_token titleToken;
-            winrt::event_token fontToken;
             winrt::event_token colorToken;
             winrt::event_token taskbarToken;
             winrt::event_token readOnlyToken;

From bb03d7e8f3018d65de290a42c8f586b66d15d0b1 Mon Sep 17 00:00:00 2001
From: Schuyler Rosefield <Rosefield@users.noreply.github.com>
Date: Thu, 28 Oct 2021 13:09:27 -0400
Subject: [PATCH 4/4] Code review: make some things const& that could be, typo.

---
 src/cascadia/TerminalApp/Pane.cpp         | 11 +++++------
 src/cascadia/TerminalApp/Pane.h           |  2 +-
 src/cascadia/TerminalApp/TerminalPage.cpp |  2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp
index 65e5d1ee43e..6950d61e57a 100644
--- a/src/cascadia/TerminalApp/Pane.cpp
+++ b/src/cascadia/TerminalApp/Pane.cpp
@@ -444,7 +444,7 @@ std::shared_ptr<Pane> Pane::NavigateDirection(const std::shared_ptr<Pane> source
     if (direction == FocusDirection::First)
     {
         // Just get the first leaf pane.
-        auto firstPane = _FindPane([](auto p) { return p->_IsLeaf(); });
+        auto firstPane = _FindPane([](const auto& p) { return p->_IsLeaf(); });
 
         // Don't need to do any movement if we are the source and target pane.
         if (firstPane == sourcePane)
@@ -601,7 +601,7 @@ std::shared_ptr<Pane> Pane::PreviousPane(const std::shared_ptr<Pane> targetPane)
 // - the parent of `pane` if pane is in this tree.
 std::shared_ptr<Pane> Pane::_FindParentOfPane(const std::shared_ptr<Pane> pane)
 {
-    return _FindPane([&](auto p) {
+    return _FindPane([&](const auto& p) {
         return p->_firstChild == pane || p->_secondChild == pane;
     });
 }
@@ -1145,7 +1145,7 @@ Controls::Grid Pane::GetRootElement()
 //   `_lastActive`, else returns this
 std::shared_ptr<Pane> Pane::GetActivePane()
 {
-    return _FindPane([](auto p) { return p->_lastActive; });
+    return _FindPane([](const auto& p) { return p->_lastActive; });
 }
 
 // Method Description:
@@ -2208,7 +2208,6 @@ std::optional<std::optional<SplitDirection>> Pane::PreCalculateCanSplit(const st
             const auto newSecondHeight = heightMinusSeparator * secondPercent;
 
             return newFirstHeight > minSize.Height && newSecondHeight > minSize.Height ? std::optional{ splitType } : std::nullopt;
-            ;
         }
     }
     else if (_IsLeaf())
@@ -2592,7 +2591,7 @@ bool Pane::_HasChild(const std::shared_ptr<Pane> child)
         return false;
     }
 
-    return WalkTree([&](auto p) {
+    return WalkTree([&](const auto& p) {
         return p->_firstChild == child || p->_secondChild == child;
     });
 }
@@ -2605,7 +2604,7 @@ bool Pane::_HasChild(const std::shared_ptr<Pane> child)
 // - A pointer to the pane with the given ID, if found.
 std::shared_ptr<Pane> Pane::FindPane(const uint32_t id)
 {
-    return _FindPane([=](auto p) { return p->_IsLeaf() && p->_id == id; });
+    return _FindPane([=](const auto& p) { return p->_IsLeaf() && p->_id == id; });
 }
 
 // Method Description:
diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h
index 1e875a98420..4646a4dad3f 100644
--- a/src/cascadia/TerminalApp/Pane.h
+++ b/src/cascadia/TerminalApp/Pane.h
@@ -184,7 +184,7 @@ class Pane : public std::enable_shared_from_this<Pane>
     template<typename F>
     std::shared_ptr<Pane> _FindPane(F f)
     {
-        return WalkTree([f](auto pane) -> std::shared_ptr<Pane> {
+        return WalkTree([f](const auto& pane) -> std::shared_ptr<Pane> {
             if (f(pane))
             {
                 return pane;
diff --git a/src/cascadia/TerminalApp/TerminalPage.cpp b/src/cascadia/TerminalApp/TerminalPage.cpp
index 5c2f65be510..bd905062ddd 100644
--- a/src/cascadia/TerminalApp/TerminalPage.cpp
+++ b/src/cascadia/TerminalApp/TerminalPage.cpp
@@ -1581,7 +1581,7 @@ namespace winrt::TerminalApp::implementation
             _RegisterTerminalEvents(newControl);
 
             _UnZoomIfNeeded();
-            tab.SplitPane(realSplitType.value(), splitSize, profile, newControl);
+            tab.SplitPane(*realSplitType, splitSize, profile, newControl);
 
             // After GH#6586, the control will no longer focus itself
             // automatically when it's finished being laid out. Manually focus