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.

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

Terminal ignores Profile termination behavior setting #16068

Closed
Avi0 opened this issue Sep 30, 2023 · 2 comments · Fixed by #16090
Closed

Terminal ignores Profile termination behavior setting #16068

Avi0 opened this issue Sep 30, 2023 · 2 comments · Fixed by #16090
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal.

Comments

@Avi0
Copy link

Avi0 commented Sep 30, 2023

Windows Terminal version

1.19.2682.0

Windows build number

10.0.22621.0

Other Software

WSL

Steps to reproduce

Set Profile termination behavior to "Close when process exits, fails, or crashes". Make profile exit with non-zero exit code. I can reproduce this in WSL, for example, by listing a non-existing file ls 123, then exit.

Expected Behavior

Tab closes silently. (Terminal 1.18 closes tab silently in this case).

Actual Behavior

This message is displayed:

[process exited with code 2 (0x00000002)]
You can now close this terminal with Ctrl+D, or press Enter to restart.
@Avi0 Avi0 added Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 30, 2023
@joaoricarte
Copy link

I am in the same terminal version and I can confirm this issue. In the previous version, this wouldn't happen.

@zadjii-msft zadjii-msft added Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface Product-Terminal The new Windows Terminal. labels Oct 2, 2023
@zadjii-msft zadjii-msft added this to the Terminal v1.20 milestone Oct 2, 2023
@zadjii-msft
Copy link
Member

zadjii-msft commented Oct 2, 2023

notes:

const auto previousConnectionState = std::exchange(_connectionState, newConnectionState);
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.
co_return;
}

When we get there as we're exiting, previousConnectionState is evaluated as... NotConnected? So we never propogated up the Connecting, Connected? weird....

oh derp
literally just above that:

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

zadjii-msft added a commit that referenced this issue Oct 2, 2023
Well, Pane doesn't _only_ care if the connection isn't entering a terminal
state. It does need to update its own state first.

Regressed in #15335

Closes #16068
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Oct 2, 2023
DHowett pushed a commit that referenced this issue Oct 3, 2023
Well, Pane doesn't _only_ care if the connection isn't entering a
terminal state. It does need to update its own state first.

Regressed in #15335

Closes #16068
DHowett pushed a commit that referenced this issue Oct 3, 2023
Well, Pane doesn't _only_ care if the connection isn't entering a
terminal state. It does need to update its own state first.

Regressed in #15335

Closes #16068

(cherry picked from commit 4145f18)
Service-Card-Id: 90731934
Service-Version: 1.19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-TerminalConnection Issues pertaining to the terminal<->backend connection interface In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants