Skip to content

Commit

Permalink
A very sensible Pane refactoring (#15232)
Browse files Browse the repository at this point in the history
It seemed dangerous to just have places all over Pane where we
manipulate the whole cadre of TermControl events. Seemed ripe for a
copypasta error. This moves that around, so there's only two methods for
messing with the TermControl callbacks: `_setupControlEvents` and
`_removeControlEvents`.

Closes: nothing. This was an off-the-cuff commit that seemed valuable.
  • Loading branch information
zadjii-msft authored Apr 25, 2023
1 parent 06dc975 commit 7e9f09f
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 24 deletions.
38 changes: 17 additions & 21 deletions src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,7 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo
_root.Children().Append(_borderFirst);
_borderFirst.Child(_control);

_connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler });
_warningBellToken = _control.WarningBell({ this, &Pane::_ControlWarningBellHandler });
_closeTerminalRequestedToken = _control.CloseTerminalRequested({ this, &Pane::_CloseTerminalRequestedHandler });
_setupControlEvents();

// Register an event with the control to have it inform us when it gains focus.
_gotFocusRevoker = _control.GotFocus(winrt::auto_revoke, { this, &Pane::_ControlGotFocusHandler });
Expand Down Expand Up @@ -104,6 +102,17 @@ Pane::Pane(std::shared_ptr<Pane> first,
});
}

void Pane::_setupControlEvents()
{
_controlEvents._ConnectionStateChanged = _control.ConnectionStateChanged(winrt::auto_revoke, { this, &Pane::_ControlConnectionStateChangedHandler });
_controlEvents._WarningBell = _control.WarningBell(winrt::auto_revoke, { this, &Pane::_ControlWarningBellHandler });
_controlEvents._CloseTerminalRequested = _control.CloseTerminalRequested(winrt::auto_revoke, { this, &Pane::_CloseTerminalRequestedHandler });
}
void Pane::_removeControlEvents()
{
_controlEvents = {};
}

// Method Description:
// - Extract the terminal settings from the current (leaf) pane's control
// to be used to create an equivalent control
Expand Down Expand Up @@ -1642,9 +1651,7 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching)
_isDefTermSession = remainingChild->_isDefTermSession;

// Add our new event handler before revoking the old one.
_connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler });
_warningBellToken = _control.WarningBell({ this, &Pane::_ControlWarningBellHandler });
_closeTerminalRequestedToken = _control.CloseTerminalRequested({ this, &Pane::_CloseTerminalRequestedHandler });
_setupControlEvents();

// Revoke the old event handlers. Remove both the handlers for the panes
// themselves closing, and remove their handlers for their controls
Expand All @@ -1658,18 +1665,14 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching)
closedChild->WalkTree([](auto p) {
if (p->_IsLeaf())
{
p->_control.ConnectionStateChanged(p->_connectionStateChangedToken);
p->_control.WarningBell(p->_warningBellToken);
p->_control.CloseTerminalRequested(p->_closeTerminalRequestedToken);
p->_removeControlEvents();
}
});
}

closedChild->Closed(closedChildClosedToken);
remainingChild->Closed(remainingChildClosedToken);
remainingChild->_control.ConnectionStateChanged(remainingChild->_connectionStateChangedToken);
remainingChild->_control.WarningBell(remainingChild->_warningBellToken);
remainingChild->_control.CloseTerminalRequested(remainingChild->_closeTerminalRequestedToken);
remainingChild->_removeControlEvents();

// If we or either of our children was focused, we want to take that
// focus from them.
Expand Down Expand Up @@ -1749,9 +1752,7 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching)
closedChild->WalkTree([](auto p) {
if (p->_IsLeaf())
{
p->_control.ConnectionStateChanged(p->_connectionStateChangedToken);
p->_control.WarningBell(p->_warningBellToken);
p->_control.CloseTerminalRequested(p->_closeTerminalRequestedToken);
p->_removeControlEvents();
}
});
}
Expand Down Expand Up @@ -2506,12 +2507,7 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitDirect
if (_IsLeaf())
{
// revoke our handler - the child will take care of the control now.
_control.ConnectionStateChanged(_connectionStateChangedToken);
_connectionStateChangedToken.value = 0;
_control.WarningBell(_warningBellToken);
_warningBellToken.value = 0;
_control.CloseTerminalRequested(_closeTerminalRequestedToken);
_closeTerminalRequestedToken.value = 0;
_removeControlEvents();

// Remove our old GotFocus handler from the control. We don't want the
// control telling us that it's now focused, we want it telling its new
Expand Down
12 changes: 9 additions & 3 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -241,11 +241,17 @@ class Pane : public std::enable_shared_from_this<Pane>
std::weak_ptr<Pane> _parentChildPath{};

bool _lastActive{ false };
winrt::event_token _connectionStateChangedToken{ 0 };
winrt::event_token _firstClosedToken{ 0 };
winrt::event_token _secondClosedToken{ 0 };
winrt::event_token _warningBellToken{ 0 };
winrt::event_token _closeTerminalRequestedToken{ 0 };

struct ControlEventTokens
{
winrt::Microsoft::Terminal::Control::TermControl::ConnectionStateChanged_revoker _ConnectionStateChanged;
winrt::Microsoft::Terminal::Control::TermControl::WarningBell_revoker _WarningBell;
winrt::Microsoft::Terminal::Control::TermControl::CloseTerminalRequested_revoker _CloseTerminalRequested;
} _controlEvents;
void _setupControlEvents();
void _removeControlEvents();

winrt::Windows::UI::Xaml::UIElement::GotFocus_revoker _gotFocusRevoker;
winrt::Windows::UI::Xaml::UIElement::LostFocus_revoker _lostFocusRevoker;
Expand Down

0 comments on commit 7e9f09f

Please sign in to comment.