From c0e4689e777218a32c67bd2c27058bb62a5ed3a1 Mon Sep 17 00:00:00 2001 From: Steve Otteson Date: Thu, 8 Dec 2022 05:29:34 -0800 Subject: [PATCH] When the terminal has exited, ctrl+D to close pane, Enter to restart terminal (#14060) When a terminal process exits (successful or not) and the profile isn't set to automatically close the pane, a new message is displayed: You can now close this terminal with ^D or Enter to restart. Ctrl+D then is able to close the pane and Enter restarts it. I originally tried to do this at the ConptyConnection layer by changing the connection state from Failed to Closed, but then that didn't work for the case where the process exited successfully but the profile isn't set to exit automatically. So, I added an event to ControlCore/TermControl that Pane watches. ControlCore watches to see if the input is Ctrl+D (0x4) and if the connection is closed or failed, and then raises the event so that Pane can close itself. As it turned out, I think this is the better place to have the logic to watch for the Ctrl+D key. Doing it at the ConptyConnection layer meant I had to parse out the key from the escaped text passed to ConptyConnection::WriteInput. ## Validation Steps Performed Tried adding lots of panes and then killing the processes outside of Terminal. Each showed the new message and I could close them with Ctrl+D or restart them with Enter. Also set a profile to never close automatically to make sure Ctrl+D would work when a process exits successfully. Closes #12849 Closes #11570 Closes #4379 --- .github/actions/spelling/allow/allow.txt | 2 + src/cascadia/TerminalApp/Pane.cpp | 29 ++++++++++- src/cascadia/TerminalApp/Pane.h | 2 + .../ConnectionStateHolder.h | 8 +++ .../TerminalConnection/ConptyConnection.cpp | 52 +++++++++++++------ .../TerminalConnection/ConptyConnection.h | 4 +- .../Resources/en-US/Resources.resw | 4 ++ src/cascadia/TerminalControl/ControlCore.cpp | 19 +++++++ src/cascadia/TerminalControl/ControlCore.h | 1 + src/cascadia/TerminalControl/ControlCore.idl | 1 + src/cascadia/TerminalControl/TermControl.h | 1 + src/cascadia/TerminalControl/TermControl.idl | 2 + 12 files changed, 106 insertions(+), 19 deletions(-) diff --git a/.github/actions/spelling/allow/allow.txt b/.github/actions/spelling/allow/allow.txt index eaa0a471190..af75e17ab2a 100644 --- a/.github/actions/spelling/allow/allow.txt +++ b/.github/actions/spelling/allow/allow.txt @@ -12,6 +12,7 @@ clickable clig CMMI copyable +CtrlDToClose cybersecurity dalet Dcs @@ -22,6 +23,7 @@ downside downsides dze dzhe +DTo EDDB EDDC Enum'd diff --git a/src/cascadia/TerminalApp/Pane.cpp b/src/cascadia/TerminalApp/Pane.cpp index b1060590d4e..ca982bcffc8 100644 --- a/src/cascadia/TerminalApp/Pane.cpp +++ b/src/cascadia/TerminalApp/Pane.cpp @@ -44,6 +44,7 @@ Pane::Pane(const Profile& profile, const TermControl& control, const bool lastFo _connectionStateChangedToken = _control.ConnectionStateChanged({ this, &Pane::_ControlConnectionStateChangedHandler }); _warningBellToken = _control.WarningBell({ this, &Pane::_ControlWarningBellHandler }); + _closeTerminalRequestedToken = _control.CloseTerminalRequested({ this, &Pane::_CloseTerminalRequestedHandler }); // On the first Pane's creation, lookup resources we'll use to theme the // Pane, including the brushed to use for the focused/unfocused border @@ -993,7 +994,7 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio // actually no longer _our_ control, and instead could be a descendant. // // When the control's new Pane takes ownership of the control, the new - // parent will register it's own event handler. That event handler will get + // parent will register its own event handler. That event handler will get // fired after this handler returns, and will properly cleanup state. if (!_IsLeaf()) { @@ -1036,6 +1037,26 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio } } +void Pane::_CloseTerminalRequestedHandler(const winrt::Windows::Foundation::IInspectable& /*sender*/, + const winrt::Windows::Foundation::IInspectable& /*args*/) +{ + std::unique_lock lock{ _createCloseLock }; + + // It's possible that this event handler started being executed, then before + // we got the lock, another thread created another child. So our control is + // actually no longer _our_ control, and instead could be a descendant. + // + // When the control's new Pane takes ownership of the control, the new + // parent will register its own event handler. That event handler will get + // fired after this handler returns, and will properly cleanup state. + if (!_IsLeaf()) + { + return; + } + + Close(); +} + winrt::fire_and_forget Pane::_playBellSound(winrt::Windows::Foundation::Uri uri) { auto weakThis{ weak_from_this() }; @@ -1586,6 +1607,7 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching) // 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 }); // Revoke the old event handlers. Remove both the handlers for the panes // themselves closing, and remove their handlers for their controls @@ -1601,6 +1623,7 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching) { p->_control.ConnectionStateChanged(p->_connectionStateChangedToken); p->_control.WarningBell(p->_warningBellToken); + p->_control.CloseTerminalRequested(p->_closeTerminalRequestedToken); } }); } @@ -1609,6 +1632,7 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching) remainingChild->Closed(remainingChildClosedToken); remainingChild->_control.ConnectionStateChanged(remainingChild->_connectionStateChangedToken); remainingChild->_control.WarningBell(remainingChild->_warningBellToken); + remainingChild->_control.CloseTerminalRequested(remainingChild->_closeTerminalRequestedToken); // If we or either of our children was focused, we want to take that // focus from them. @@ -1690,6 +1714,7 @@ void Pane::_CloseChild(const bool closeFirst, const bool isDetaching) { p->_control.ConnectionStateChanged(p->_connectionStateChangedToken); p->_control.WarningBell(p->_warningBellToken); + p->_control.CloseTerminalRequested(p->_closeTerminalRequestedToken); } }); } @@ -2448,6 +2473,8 @@ std::pair, std::shared_ptr> Pane::_Split(SplitDirect _connectionStateChangedToken.value = 0; _control.WarningBell(_warningBellToken); _warningBellToken.value = 0; + _control.CloseTerminalRequested(_closeTerminalRequestedToken); + _closeTerminalRequestedToken.value = 0; // 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 diff --git a/src/cascadia/TerminalApp/Pane.h b/src/cascadia/TerminalApp/Pane.h index c81bd3fd096..70f7fbfd7a2 100644 --- a/src/cascadia/TerminalApp/Pane.h +++ b/src/cascadia/TerminalApp/Pane.h @@ -237,6 +237,7 @@ class Pane : public std::enable_shared_from_this winrt::event_token _firstClosedToken{ 0 }; winrt::event_token _secondClosedToken{ 0 }; winrt::event_token _warningBellToken{ 0 }; + winrt::event_token _closeTerminalRequestedToken{ 0 }; winrt::Windows::UI::Xaml::UIElement::GotFocus_revoker _gotFocusRevoker; winrt::Windows::UI::Xaml::UIElement::LostFocus_revoker _lostFocusRevoker; @@ -290,6 +291,7 @@ class Pane : public std::enable_shared_from_this const winrt::Windows::UI::Xaml::RoutedEventArgs& e); void _ControlLostFocusHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::UI::Xaml::RoutedEventArgs& e); + void _CloseTerminalRequestedHandler(const winrt::Windows::Foundation::IInspectable& sender, const winrt::Windows::Foundation::IInspectable& /*args*/); std::pair _CalcChildrenSizes(const float fullSize) const; SnapChildrenSizeResult _CalcSnappedChildrenSizes(const bool widthOrHeight, const float fullSize) const; diff --git a/src/cascadia/TerminalConnection/ConnectionStateHolder.h b/src/cascadia/TerminalConnection/ConnectionStateHolder.h index 947e16788f3..f037b6e9c9c 100644 --- a/src/cascadia/TerminalConnection/ConnectionStateHolder.h +++ b/src/cascadia/TerminalConnection/ConnectionStateHolder.h @@ -86,6 +86,14 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation return _isStateOneOf(ConnectionState::Connected); } + void _resetConnectionState() + { + { + std::lock_guard stateLock{ _stateMutex }; + _connectionState = ConnectionState::NotConnected; + } + } + private: std::atomic _connectionState{ ConnectionState::NotConnected }; mutable std::mutex _stateMutex; diff --git a/src/cascadia/TerminalConnection/ConptyConnection.cpp b/src/cascadia/TerminalConnection/ConptyConnection.cpp index 76922788845..659cfc4bd47 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.cpp +++ b/src/cascadia/TerminalConnection/ConptyConnection.cpp @@ -205,8 +205,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation const HANDLE hServerProcess, const HANDLE hClientProcess, TERMINAL_STARTUP_INFO startupInfo) : - _initialRows{ 25 }, - _initialCols{ 80 }, + _rows{ 25 }, + _cols{ 80 }, _guid{ Utils::CreateGuid() }, _inPipe{ hIn }, _outPipe{ hOut } @@ -268,8 +268,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _commandline = winrt::unbox_value_or(settings.TryLookup(L"commandline").try_as(), _commandline); _startingDirectory = winrt::unbox_value_or(settings.TryLookup(L"startingDirectory").try_as(), _startingDirectory); _startingTitle = winrt::unbox_value_or(settings.TryLookup(L"startingTitle").try_as(), _startingTitle); - _initialRows = winrt::unbox_value_or(settings.TryLookup(L"initialRows").try_as(), _initialRows); - _initialCols = winrt::unbox_value_or(settings.TryLookup(L"initialCols").try_as(), _initialCols); + _rows = winrt::unbox_value_or(settings.TryLookup(L"initialRows").try_as(), _rows); + _cols = winrt::unbox_value_or(settings.TryLookup(L"initialCols").try_as(), _cols); _guid = winrt::unbox_value_or(settings.TryLookup(L"guid").try_as(), _guid); _environment = settings.TryLookup(L"environment").try_as(); if constexpr (Feature_VtPassthroughMode::IsEnabled()) @@ -307,9 +307,16 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void ConptyConnection::Start() try { + bool usingExistingBuffer = false; + if (_isStateAtOrBeyond(ConnectionState::Closed)) + { + _resetConnectionState(); + usingExistingBuffer = true; + } + _transitionToState(ConnectionState::Connecting); - const til::size dimensions{ gsl::narrow(_initialCols), gsl::narrow(_initialRows) }; + const til::size dimensions{ gsl::narrow(_cols), gsl::narrow(_rows) }; // If we do not have pipes already, then this is a fresh connection... not an inbound one that is a received // handoff from an already-started PTY process. @@ -317,6 +324,16 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation { DWORD flags = PSEUDOCONSOLE_RESIZE_QUIRK | PSEUDOCONSOLE_WIN32_INPUT_MODE; + // If we're using an existing buffer, we want the new connection + // to reuse the existing cursor. When not setting this flag, the + // PseudoConsole sends a clear screen VT code which our renderer + // interprets into making all the previous lines be outside the + // current viewport. + if (usingExistingBuffer) + { + flags |= PSEUDOCONSOLE_INHERIT_CURSOR; + } + if constexpr (Feature_VtPassthroughMode::IsEnabled()) { if (_passthroughMode) @@ -440,6 +457,9 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation winrt::hstring exitText{ fmt::format(std::wstring_view{ RS_(L"ProcessExited") }, fmt::format(_errorFormat, status)) }; _TerminalOutputHandlers(L"\r\n"); _TerminalOutputHandlers(exitText); + _TerminalOutputHandlers(L"\r\n"); + _TerminalOutputHandlers(RS_(L"CtrlDToClose")); + _TerminalOutputHandlers(L"\r\n"); } CATCH_LOG(); } @@ -492,15 +512,11 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void ConptyConnection::Resize(uint32_t rows, uint32_t columns) { - // If we haven't started connecting at all, it's still fair to update - // the initial rows and columns before we set things up. - if (!_isStateAtOrBeyond(ConnectionState::Connecting)) - { - _initialRows = rows; - _initialCols = columns; - } - // Otherwise, we can really only dispatch a resize if we're already connected. - else if (_isConnected()) + // Always keep these in case we ever want to disconnect/restart + _rows = rows; + _cols = columns; + + if (_isConnected()) { THROW_IF_FAILED(ConptyResizePseudoConsole(_hPC.get(), { Utils::ClampToShortMax(columns, 1), Utils::ClampToShortMax(rows, 1) })); } @@ -548,7 +564,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void ConptyConnection::Close() noexcept try { - if (_transitionToState(ConnectionState::Closing)) + const bool isClosing = _transitionToState(ConnectionState::Closing); + if (isClosing || _isStateAtOrBeyond(ConnectionState::Closed)) { // EXIT POINT @@ -579,7 +596,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation _hOutputThread.reset(); } - _transitionToState(ConnectionState::Closed); + if (isClosing) + { + _transitionToState(ConnectionState::Closed); + } } } CATCH_LOG() diff --git a/src/cascadia/TerminalConnection/ConptyConnection.h b/src/cascadia/TerminalConnection/ConptyConnection.h index 601141d9852..904eb50ff73 100644 --- a/src/cascadia/TerminalConnection/ConptyConnection.h +++ b/src/cascadia/TerminalConnection/ConptyConnection.h @@ -72,8 +72,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation void _indicateExitWithStatus(unsigned int status) noexcept; void _ClientTerminated() noexcept; - til::CoordType _initialRows{}; - til::CoordType _initialCols{}; + til::CoordType _rows{}; + til::CoordType _cols{}; uint64_t _initialParentHwnd{ 0 }; hstring _commandline{}; hstring _startingDirectory{}; diff --git a/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw b/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw index 3395cb7dd55..3532e310491 100644 --- a/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw +++ b/src/cascadia/TerminalConnection/Resources/en-US/Resources.resw @@ -207,6 +207,10 @@ [process exited with code {0}] The first argument {0} is the error code of the process. When there is no error, the number ZERO will be displayed. + + You can now close this terminal with Ctrl+D, or press Enter to restart. + "Ctrl+D" and "Enter" represent keys the user will press (control+D and Enter). + [error {0} when launching `{1}'] The first argument {0} is the error code. The second argument {1} is the user-specified path to a program. diff --git a/src/cascadia/TerminalControl/ControlCore.cpp b/src/cascadia/TerminalControl/ControlCore.cpp index 1fe34b73cb5..d1388214a5f 100644 --- a/src/cascadia/TerminalControl/ControlCore.cpp +++ b/src/cascadia/TerminalControl/ControlCore.cpp @@ -381,6 +381,25 @@ namespace winrt::Microsoft::Terminal::Control::implementation const WORD scanCode, const ::Microsoft::Terminal::Core::ControlKeyStates modifiers) { + const wchar_t CtrlD = 0x4; + const wchar_t Enter = '\r'; + + if (_connection.State() >= winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::Closed) + { + if (ch == CtrlD) + { + _CloseTerminalRequestedHandlers(*this, nullptr); + return true; + } + + if (ch == Enter) + { + _connection.Close(); + _connection.Start(); + return true; + } + } + if (ch == L'\x3') // Ctrl+C or Ctrl+Break { _handleControlC(); diff --git a/src/cascadia/TerminalControl/ControlCore.h b/src/cascadia/TerminalControl/ControlCore.h index 7d5fa75dd23..890e9ec3d51 100644 --- a/src/cascadia/TerminalControl/ControlCore.h +++ b/src/cascadia/TerminalControl/ControlCore.h @@ -232,6 +232,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation TYPED_EVENT(ShowWindowChanged, IInspectable, Control::ShowWindowArgs); TYPED_EVENT(UpdateSelectionMarkers, IInspectable, Control::UpdateSelectionMarkersEventArgs); TYPED_EVENT(OpenHyperlink, IInspectable, Control::OpenHyperlinkEventArgs); + TYPED_EVENT(CloseTerminalRequested, IInspectable, IInspectable); // clang-format on private: diff --git a/src/cascadia/TerminalControl/ControlCore.idl b/src/cascadia/TerminalControl/ControlCore.idl index a65384e2bc2..a8df501469b 100644 --- a/src/cascadia/TerminalControl/ControlCore.idl +++ b/src/cascadia/TerminalControl/ControlCore.idl @@ -161,5 +161,6 @@ namespace Microsoft.Terminal.Control event Windows.Foundation.TypedEventHandler ShowWindowChanged; event Windows.Foundation.TypedEventHandler UpdateSelectionMarkers; event Windows.Foundation.TypedEventHandler OpenHyperlink; + event Windows.Foundation.TypedEventHandler CloseTerminalRequested; }; } diff --git a/src/cascadia/TerminalControl/TermControl.h b/src/cascadia/TerminalControl/TermControl.h index bed685ede24..590780e7524 100644 --- a/src/cascadia/TerminalControl/TermControl.h +++ b/src/cascadia/TerminalControl/TermControl.h @@ -148,6 +148,7 @@ namespace winrt::Microsoft::Terminal::Control::implementation PROJECTED_FORWARDED_TYPED_EVENT(SetTaskbarProgress, IInspectable, IInspectable, _core, TaskbarProgressChanged); PROJECTED_FORWARDED_TYPED_EVENT(ConnectionStateChanged, IInspectable, IInspectable, _core, ConnectionStateChanged); PROJECTED_FORWARDED_TYPED_EVENT(ShowWindowChanged, IInspectable, Control::ShowWindowArgs, _core, ShowWindowChanged); + PROJECTED_FORWARDED_TYPED_EVENT(CloseTerminalRequested, IInspectable, IInspectable, _core, CloseTerminalRequested); PROJECTED_FORWARDED_TYPED_EVENT(PasteFromClipboard, IInspectable, Control::PasteFromClipboardEventArgs, _interactivity, PasteFromClipboard); diff --git a/src/cascadia/TerminalControl/TermControl.idl b/src/cascadia/TerminalControl/TermControl.idl index a287989d19f..044bfb0a6b1 100644 --- a/src/cascadia/TerminalControl/TermControl.idl +++ b/src/cascadia/TerminalControl/TermControl.idl @@ -52,6 +52,8 @@ namespace Microsoft.Terminal.Control event Windows.Foundation.TypedEventHandler ShowWindowChanged; + event Windows.Foundation.TypedEventHandler CloseTerminalRequested; + Boolean CopySelectionToClipboard(Boolean singleLine, Windows.Foundation.IReference formats); void PasteTextFromClipboard(); void SelectAll();