A very sensible Pane refactoring (#15232)

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.
This commit is contained in:
Mike Griese
2023-04-25 15:58:13 -05:00
committed by GitHub
parent 06dc975a0e
commit 7e9f09f495
2 changed files with 26 additions and 24 deletions

View File

@@ -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 });
@@ -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
@@ -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
@@ -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.
@@ -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();
}
});
}
@@ -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

View File

@@ -241,11 +241,17 @@ private:
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;