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.

Sign up for GitHub

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

Ignore closeOnExit when a conn. moves from Connecting to Failed #10263

Merged
merged 1 commit into from
May 28, 2021
Merged

Ignore closeOnExit when a conn. moves from Connecting to Failed #10263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
11 changes: 11 additions & 0 deletions src/cascadia/TerminalApp/Pane.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,13 +329,22 @@ void Pane::_ControlConnectionStateChangedHandler(const winrt::Windows::Foundatio
}

const auto newConnectionState = _control.ConnectionState();
const auto previousConnectionState = std::exchange(_connectionState, newConnectionState);

if (newConnectionState < ConnectionState::Closed)
{
// Pane doesn't care if the connection isn't entering a terminal state.
return;
}

if (previousConnectionState < ConnectionState::Connected && newConnectionState >= ConnectionState::Failed)
{
// A failure to complete the connection (before it has _connected_) is not covered by "closeOnExit".
// This is to prevent a misconfiguration (closeOnExit: always, startingDirectory: garbage) resulting
// in Terminal flashing open and immediately closed.
return;
}

const auto settings{ winrt::TerminalApp::implementation::AppLogic::CurrentAppSettings() };
auto paneProfile = settings.FindProfile(_profile.value());
if (paneProfile)
Expand Down Expand Up @@ -709,6 +718,7 @@ void Pane::_CloseChild(const bool closeFirst)

// take the control, profile and id of the pane that _wasn't_ closed.
_control = remainingChild->_control;
_connectionState = remainingChild->_connectionState;
_profile = remainingChild->_profile;
_id = remainingChild->Id();

Expand Down Expand Up @@ -1476,6 +1486,7 @@ std::pair<std::shared_ptr<Pane>, std::shared_ptr<Pane>> Pane::_Split(SplitState
// Move our control, guid into the first one.
// Move the new guid, control into the second.
_firstChild = std::make_shared<Pane>(_profile.value(), _control);
_firstChild->_connectionState = std::exchange(_connectionState, ConnectionState::NotConnected);
Copy link
Member

Choose a reason for hiding this comment

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

Is it just me or is it kinda hacky that this isn't part of the/a constructor?

Copy link
Member Author

Choose a reason for hiding this comment

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

I rather think the design of Pane leaves a lot to be desired; there are a lot of properties that "move" between parent and leaf panes. The ideal design is one that doesn't require us to move state at all. I'd rather not spend too much effort on an already-bad solution 😄

_profile = std::nullopt;
_control = { nullptr };
_secondChild = std::make_shared<Pane>(profile, control);
Expand Down
1 change: 1 addition & 0 deletions src/cascadia/TerminalApp/Pane.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ class Pane : public std::enable_shared_from_this<Pane>
winrt::Windows::UI::Xaml::Controls::Grid _root{};
winrt::Windows::UI::Xaml::Controls::Border _border{};
winrt::Microsoft::Terminal::Control::TermControl _control{ nullptr };
winrt::Microsoft::Terminal::TerminalConnection::ConnectionState _connectionState{ winrt::Microsoft::Terminal::TerminalConnection::ConnectionState::NotConnected };
static winrt::Windows::UI::Xaml::Media::SolidColorBrush s_focusedBorderBrush;
static winrt::Windows::UI::Xaml::Media::SolidColorBrush s_unfocusedBorderBrush;

Expand Down
2 changes: 2 additions & 0 deletions src/cascadia/TerminalConnection/ConptyConnection.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,8 @@ namespace winrt::Microsoft::Terminal::TerminalConnection::implementation
void ConptyConnection::Start()
try
{
_transitionToState(ConnectionState::Connecting);

if (!_inPipe)
{
const COORD dimensions{ gsl::narrow_cast<SHORT>(_initialCols), gsl::narrow_cast<SHORT>(_initialRows) };
Expand Down