Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up Pane #2494

Merged
merged 4 commits into from
Aug 28, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/cascadia/TerminalApp/App.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1493,8 +1493,7 @@ namespace winrt::TerminalApp::implementation
const int focusedTabIndex = _GetFocusedTabIndex();
auto focusedTab = _tabs[focusedTabIndex];

const auto canSplit = splitType == Pane::SplitState::Horizontal ? focusedTab->CanAddHorizontalSplit() :
focusedTab->CanAddVerticalSplit();
const auto canSplit = focusedTab->CanSplitPane(splitType);

if (!canSplit)
{
Expand All @@ -1506,8 +1505,7 @@ namespace winrt::TerminalApp::implementation
// Hookup our event handlers to the new terminal
_RegisterTerminalEvents(newControl, focusedTab);

return splitType == Pane::SplitState::Horizontal ? focusedTab->AddHorizontalSplit(realGuid, newControl) :
focusedTab->AddVerticalSplit(realGuid, newControl);
focusedTab->SplitPane(splitType, realGuid, newControl);
}

// Method Description:
Expand Down
184 changes: 63 additions & 121 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -164,26 +164,26 @@ bool Pane::ResizePane(const Direction& direction)
{
return _Resize(direction);
}
else

// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the resize.
// For each child, if it has a focused descendant, try having that child
// handle the resize.
// If the child wasn't able to handle the resize, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to resize. Otherwise, just return false, as we couldn't handle it
// either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pattern ("find the non-leaf's child, if it has one") appears a lot. Is there a way to pull it out into a helper? I know that we do slightly different things with each one, but the pattern looks the same:

if(!<x>.leaf && <x>.child) {
    make <x> do <A>, or do <_A> ourselves
}

perhaps we can:

auto x = _findFirstChildPaneMatchingThoseCriteriaOrNot();
return x.has_value() && x->A() || _A();

{
// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the resize.
// For each child, if it has a focused descendant, try having that child
// handle the resize.
// If the child wasn't able to handle the resize, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to resize. Otherwise, just return false, as we couldn't handle it
// either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
{
return _firstChild->ResizePane(direction) || _Resize(direction);
}
else if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->ResizePane(direction) || _Resize(direction);
}
return _firstChild->ResizePane(direction) || _Resize(direction);
}

if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->ResizePane(direction) || _Resize(direction);
}

return false;
}

Expand Down Expand Up @@ -253,26 +253,26 @@ bool Pane::NavigateFocus(const Direction& direction)
{
return _NavigateFocus(direction);
}
else

// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the focus move.
// For each child, if it has a focused descendant, try having that child
// handle the focus move.
// If the child wasn't able to handle the focus move, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to move focus into our other child. Otherwise, just return false, as
// we couldn't handle it either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
{
// If neither of our children were the focused leaf, then recurse into
// our children and see if they can handle the focus move.
// For each child, if it has a focused descendant, try having that child
// handle the focus move.
// If the child wasn't able to handle the focus move, it's possible that
// there were no descendants with a separator the correct direction. If
// our separator _is_ the correct direction, then we should be the pane
// to move focus into our other child. Otherwise, just return false, as
// we couldn't handle it either.
if ((!_firstChild->_IsLeaf()) && _firstChild->_HasFocusedChild())
{
return _firstChild->NavigateFocus(direction) || _NavigateFocus(direction);
}
else if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->NavigateFocus(direction) || _NavigateFocus(direction);
}
return _firstChild->NavigateFocus(direction) || _NavigateFocus(direction);
}

if ((!_secondChild->_IsLeaf()) && _secondChild->_HasFocusedChild())
{
return _secondChild->NavigateFocus(direction) || _NavigateFocus(direction);
}

return false;
}

Expand Down Expand Up @@ -348,15 +348,13 @@ std::shared_ptr<Pane> Pane::GetFocusedPane()
{
return _lastFocused ? shared_from_this() : nullptr;
}
else

auto firstFocused = _firstChild->GetFocusedPane();
if (firstFocused != nullptr)
{
auto firstFocused = _firstChild->GetFocusedPane();
if (firstFocused != nullptr)
{
return firstFocused;
}
return _secondChild->GetFocusedPane();
return firstFocused;
}
return _secondChild->GetFocusedPane();
}

// Method Description:
Expand Down Expand Up @@ -778,110 +776,58 @@ void Pane::_ApplySplitDefinitions()
}

// Method Description:
// - Determines whether the pane can be split vertically
// - Determines whether the pane can be split
// Arguments:
// - splitType: what type of split we want to create.
// Return Value:
// - True if the pane can be split vertically. False otherwise.
bool Pane::CanSplitVertical()
// - True if the pane can be split. False otherwise.
bool Pane::CanSplit(SplitState splitType)
{
if (!_IsLeaf())
if (_IsLeaf())
{
if (_firstChild->_HasFocusedChild())
{
return _firstChild->CanSplitVertical();
}
else if (_secondChild->_HasFocusedChild())
{
return _secondChild->CanSplitVertical();
}

return false;
return _CanSplit(splitType);
}

return _CanSplit(SplitState::Vertical);
}

// Method Description:
// - Vertically split the focused pane in our tree of panes, and place the given
// TermControl into the newly created pane. If we're the focused pane, then
// we'll create two new children, and place them side-by-side in our Grid.
// Arguments:
// - profile: The profile GUID to associate with the newly created pane.
// - control: A TermControl to use in the new pane.
// Return Value:
// - <none>
void Pane::SplitVertical(const GUID& profile, const TermControl& control)
{
// If we're not the leaf, recurse into our children to split them.
if (!_IsLeaf())
if (_firstChild->_HasFocusedChild())
{
if (_firstChild->_HasFocusedChild())
{
_firstChild->SplitVertical(profile, control);
}
else if (_secondChild->_HasFocusedChild())
{
_secondChild->SplitVertical(profile, control);
}

return;
return _firstChild->CanSplit(splitType);
}

_Split(SplitState::Vertical, profile, control);
}

// Method Description:
// - Determines whether the pane can be split horizontally
// Arguments:
// - splitType: what type of split we want to create.
// Return Value:
// - True if the pane can be split horizontally. False otherwise.
bool Pane::CanSplitHorizontal()
{
if (!_IsLeaf())
if (_secondChild->_HasFocusedChild())
{
if (_firstChild->_HasFocusedChild())
{
return _firstChild->CanSplitHorizontal();
}
else if (_secondChild->_HasFocusedChild())
{
return _secondChild->CanSplitHorizontal();
}

return false;
return _secondChild->CanSplit(splitType);
}

return _CanSplit(SplitState::Horizontal);
return false;
}

// Method Description:
// - Horizontally split the focused pane in our tree of panes, and place the given
// - Split the focused pane in our tree of panes, and place the given
// TermControl into the newly created pane. If we're the focused pane, then
// we'll create two new children, and place them side-by-side in our Grid.
// Arguments:
// - splitType: what type of split we want to create.
// - profile: The profile GUID to associate with the newly created pane.
// - control: A TermControl to use in the new pane.
// Return Value:
// - <none>
void Pane::SplitHorizontal(const GUID& profile, const TermControl& control)
void Pane::Split(SplitState splitType, const GUID& profile, const TermControl& control)
{
if (!_IsLeaf())
{
if (_firstChild->_HasFocusedChild())
{
_firstChild->SplitHorizontal(profile, control);
_firstChild->Split(splitType, profile, control);
}
else if (_secondChild->_HasFocusedChild())
{
_secondChild->SplitHorizontal(profile, control);
_secondChild->Split(splitType, profile, control);
}

return;
}

_Split(SplitState::Horizontal, profile, control);
_Split(splitType, profile, control);
}

// Method Description:
Expand All @@ -892,8 +838,6 @@ void Pane::SplitHorizontal(const GUID& profile, const TermControl& control)
// - True if the pane can be split. False otherwise.
bool Pane::_CanSplit(SplitState splitType)
{
const bool changeWidth = _splitState == SplitState::Vertical;

const Size actualSize{ gsl::narrow_cast<float>(_root.ActualWidth()),
gsl::narrow_cast<float>(_root.ActualHeight()) };

Expand Down Expand Up @@ -1006,14 +950,12 @@ Size Pane::_GetMinSize() const
{
return _control.MinimumSize();
}
else
{
const auto firstSize = _firstChild->_GetMinSize();
const auto secondSize = _secondChild->_GetMinSize();
const auto newWidth = firstSize.Width + secondSize.Width + (_splitState == SplitState::Vertical ? PaneSeparatorSize : 0);
const auto newHeight = firstSize.Height + secondSize.Height + (_splitState == SplitState::Horizontal ? PaneSeparatorSize : 0);
return { newWidth, newHeight };
}

const auto firstSize = _firstChild->_GetMinSize();
const auto secondSize = _secondChild->_GetMinSize();
const auto newWidth = firstSize.Width + secondSize.Width + (_splitState == SplitState::Vertical ? PaneSeparatorSize : 0);
const auto newHeight = firstSize.Height + secondSize.Height + (_splitState == SplitState::Horizontal ? PaneSeparatorSize : 0);
return { newWidth, newHeight };
}

DEFINE_EVENT(Pane, Closed, _closedHandlers, ConnectionClosedEventArgs);
7 changes: 2 additions & 5 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,8 @@ class Pane : public std::enable_shared_from_this<Pane>
bool ResizePane(const winrt::TerminalApp::Direction& direction);
bool NavigateFocus(const winrt::TerminalApp::Direction& direction);

bool CanSplitHorizontal();
void SplitHorizontal(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);

bool CanSplitVertical();
void SplitVertical(const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
bool CanSplit(SplitState splitType);
void Split(SplitState splitType, const GUID& profile, const winrt::Microsoft::Terminal::TerminalControl::TermControl& control);

void Close();

Expand Down
37 changes: 9 additions & 28 deletions src/cascadia/TerminalApp/Tab.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,47 +204,28 @@ void Tab::Scroll(const int delta)
}

// Method Description:
// - Determines whether the focused pane has sufficient space to be split vertically.
// Return Value:
// - True if the focused pane can be split horizontally. False otherwise.
bool Tab::CanAddVerticalSplit()
{
return _rootPane->CanSplitVertical();
}

// Method Description:
// - Vertically split the focused pane in our tree of panes, and place the
// given TermControl into the newly created pane.
// - Determines whether the focused pane has sufficient space to be split.
// Arguments:
// - profile: The profile GUID to associate with the newly created pane.
// - control: A TermControl to use in the new pane.
// Return Value:
// - <none>
void Tab::AddVerticalSplit(const GUID& profile, TermControl& control)
{
_rootPane->SplitVertical(profile, control);
}

// Method Description:
// - Determines whether the focused pane has sufficient space to be split horizontally.
// - splitType: The type of split we want to create.
// Return Value:
// - True if the focused pane can be split horizontally. False otherwise.
bool Tab::CanAddHorizontalSplit()
// - True if the focused pane can be split. False otherwise.
bool Tab::CanSplitPane(Pane::SplitState splitType)
{
return _rootPane->CanSplitHorizontal();
return _rootPane->CanSplit(splitType);
}

// Method Description:
// - Horizontally split the focused pane in our tree of panes, and place the
// - Split the focused pane in our tree of panes, and place the
// given TermControl into the newly created pane.
// Arguments:
// - splitType: The type of split we want to create.
// - profile: The profile GUID to associate with the newly created pane.
// - control: A TermControl to use in the new pane.
// Return Value:
// - <none>
void Tab::AddHorizontalSplit(const GUID& profile, TermControl& control)
void Tab::SplitPane(Pane::SplitState splitType, const GUID& profile, TermControl& control)
{
_rootPane->SplitHorizontal(profile, control);
_rootPane->Split(splitType, profile, control);
}

// Method Description:
Expand Down
7 changes: 3 additions & 4 deletions src/cascadia/TerminalApp/Tab.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,9 @@ class Tab
void SetFocused(const bool focused);

void Scroll(const int delta);
bool CanAddVerticalSplit();
void AddVerticalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control);
bool CanAddHorizontalSplit();
void AddHorizontalSplit(const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control);

bool CanSplitPane(Pane::SplitState splitType);
void SplitPane(Pane::SplitState splitType, const GUID& profile, winrt::Microsoft::Terminal::TerminalControl::TermControl& control);

void UpdateFocus();
void UpdateIcon(const winrt::hstring iconPath);
Expand Down