Skip to content

Commit

Permalink
PRE-MERGE #14060 When the terminal has exited, ctrl+D to close pane, …
Browse files Browse the repository at this point in the history
…Enter to restart terminal
  • Loading branch information
carlos-zamora committed Oct 25, 2022
2 parents d42b03f + 62888a5 commit f07fa60
Show file tree
Hide file tree
Showing 11 changed files with 104 additions and 19 deletions.
29 changes: 28 additions & 1 deletion src/cascadia/TerminalApp/Pane.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())
{
Expand Down Expand Up @@ -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() };
Expand Down Expand Up @@ -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
Expand All @@ -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);
}
});
}
Expand All @@ -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.
Expand Down Expand Up @@ -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);
}
});
}
Expand Down Expand Up @@ -2448,6 +2473,8 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> 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
Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalApp/Pane.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ class Pane : public std::enable_shared_from_this<Pane>
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;
Expand Down Expand Up @@ -290,6 +291,7 @@ class Pane : public std::enable_shared_from_this<Pane>
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<float, float> _CalcChildrenSizes(const float fullSize) const;
SnapChildrenSizeResult _CalcSnappedChildrenSizes(const bool widthOrHeight, const float fullSize) const;
Expand Down
8 changes: 8 additions & 0 deletions src/cascadia/TerminalConnection/ConnectionStateHolder.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
return _isStateOneOf(ConnectionState::Connected);
}

void _resetConnectionState()
{
{
std::lock_guard<std::mutex> stateLock{ _stateMutex };
_connectionState = ConnectionState::NotConnected;
}
}

private:
std::atomic<ConnectionState> _connectionState{ ConnectionState::NotConnected };
mutable std::mutex _stateMutex;
Expand Down
52 changes: 36 additions & 16 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down Expand Up @@ -267,8 +267,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
_commandline = winrt::unbox_value_or<winrt::hstring>(settings.TryLookup(L"commandline").try_as<Windows::Foundation::IPropertyValue>(), _commandline);
_startingDirectory = winrt::unbox_value_or<winrt::hstring>(settings.TryLookup(L"startingDirectory").try_as<Windows::Foundation::IPropertyValue>(), _startingDirectory);
_startingTitle = winrt::unbox_value_or<winrt::hstring>(settings.TryLookup(L"startingTitle").try_as<Windows::Foundation::IPropertyValue>(), _startingTitle);
_initialRows = winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialRows").try_as<Windows::Foundation::IPropertyValue>(), _initialRows);
_initialCols = winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialCols").try_as<Windows::Foundation::IPropertyValue>(), _initialCols);
_rows = winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialRows").try_as<Windows::Foundation::IPropertyValue>(), _rows);
_cols = winrt::unbox_value_or<uint32_t>(settings.TryLookup(L"initialCols").try_as<Windows::Foundation::IPropertyValue>(), _cols);
_guid = winrt::unbox_value_or<winrt::guid>(settings.TryLookup(L"guid").try_as<Windows::Foundation::IPropertyValue>(), _guid);
_environment = settings.TryLookup(L"environment").try_as<Windows::Foundation::Collections::ValueSet>();
if constexpr (Feature_VtPassthroughMode::IsEnabled())
Expand Down Expand Up @@ -301,16 +301,33 @@ 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<til::CoordType>(_initialCols), gsl::narrow<til::CoordType>(_initialRows) };
const til::size dimensions{ gsl::narrow<til::CoordType>(_cols), gsl::narrow<til::CoordType>(_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.
if (!_inPipe)
{
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)
Expand Down Expand Up @@ -434,6 +451,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();
}
Expand Down Expand Up @@ -486,15 +506,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) }));
}
Expand Down Expand Up @@ -542,7 +558,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

Expand Down Expand Up @@ -573,7 +590,10 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
_hOutputThread.reset();
}

_transitionToState(ConnectionState::Closed);
if (isClosing)
{
_transitionToState(ConnectionState::Closed);
}
}
}
CATCH_LOG()
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalConnection/ConptyConnection.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,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{};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,10 @@
<value>[process exited with code {0}]</value>
<comment>The first argument {0} is the error code of the process. When there is no error, the number ZERO will be displayed. </comment>
</data>
<data name="CtrlDToClose" xml:space="preserve">
<value>You can now close this terminal with ^D or Enter to restart.</value>
<comment>{Locked="^D","Enter"} These represent keys the user will press (control+D and Enter).</comment>
</data>
<data name="ProcessFailedToLaunch" xml:space="preserve">
<value>[error {0} when launching `{1}']</value>
<comment>The first argument {0} is the error code. The second argument {1} is the user-specified path to a program.
Expand Down
19 changes: 19 additions & 0 deletions src/cascadia/TerminalControl/ControlCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -401,6 +401,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;
}
}

return _terminal->SendCharEvent(ch, scanCode, modifiers);
}

Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/ControlCore.idl
Original file line number Diff line number Diff line change
Expand Up @@ -161,5 +161,6 @@ namespace Microsoft.Terminal.Control
event Windows.Foundation.TypedEventHandler<Object, ShowWindowArgs> ShowWindowChanged;
event Windows.Foundation.TypedEventHandler<Object, UpdateSelectionMarkersEventArgs> UpdateSelectionMarkers;
event Windows.Foundation.TypedEventHandler<Object, OpenHyperlinkEventArgs> OpenHyperlink;
event Windows.Foundation.TypedEventHandler<Object, Object> CloseTerminalRequested;
};
}
1 change: 1 addition & 0 deletions src/cascadia/TerminalControl/TermControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalControl/TermControl.idl
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ namespace Microsoft.Terminal.Control

event Windows.Foundation.TypedEventHandler<Object, ShowWindowArgs> ShowWindowChanged;

event Windows.Foundation.TypedEventHandler<Object, Object> CloseTerminalRequested;

Boolean CopySelectionToClipboard(Boolean singleLine, Windows.Foundation.IReference<CopyFormat> formats);
void PasteTextFromClipboard();
void SelectAll();
Expand Down

0 comments on commit f07fa60

Please sign in to comment.