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

Make exit code behavior more disacoverable #10798

Closed
superm1 opened this issue Jul 26, 2021 · 10 comments · Fixed by #11139
Closed

Make exit code behavior more disacoverable #10798

superm1 opened this issue Jul 26, 2021 · 10 comments · Fixed by #11139
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.

Comments

@superm1
Copy link

superm1 commented Jul 26, 2021

Description of the new feature/enhancement

When a process exits with a return code while closing a terminal, the following message shows up (assuming exit 130)

[process exited with code 130]

It's difficult to know what this means, especially if it's while closing a pane. If a process returns an exit code that is non zero, I can't figure out how to get rid of that pane now. Example:
image

This inevitably leads eventually to #5214

Proposed technical implementation details (optional)

Add a clearer message why this happened, and how to change it.

Process exited with code 130, to adjust what happens with non-zero exit codes visit Settings->Foo->bar

Then in the GUI add a new option that will control adding

"closeOnExit": "always"
@superm1 superm1 added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Jul 26, 2021
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jul 26, 2021
@skyline75489
Copy link
Collaborator

Add a clearer message why this happened, and how to change it.

Cool idea :) /cc @cinnamon-msft

Then in the GUI add a new option that will control adding

This exists already.

@superm1
Copy link
Author

superm1 commented Jul 27, 2021

This exists already.

TIL. It's on a per-OS-profile basis.

@zadjii-msft zadjii-msft added the Product-Terminal The new Windows Terminal. label Jul 28, 2021
@zadjii-msft
Copy link
Member

I'm okay with adding something like

to adjust what happens with non-zero exit codes visit Settings->Foo->bar. Read more at <link-to-the-docs....>

But only in the scenario where it's only displayed the first time it happens, because otherwise I feel like it just adds noise. Fortunately, with #8324, we can actually do this. Thanks for the idea!

@zadjii-msft zadjii-msft added Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. and removed Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Aug 2, 2021
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 2, 2021
@zadjii-msft zadjii-msft added this to the Terminal Backlog milestone Aug 2, 2021
@zadjii-msft zadjii-msft removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Aug 5, 2021
@Toxic-Waste-
Copy link

Toxic-Waste- commented Aug 6, 2021

it's this by design? I have ran into this issue aswell, I use terminal to connect to multiple servers, and especially when I want to close a pane I split before to check something extra, I want the pane to close when I exit the session.

Now it seems to be stuck and I can't get my screenspace back. I also have no way of closing that pane without closing the entire tab, that can't be by design?

Example (obfuscated IP and username information for obvious reasons):
pane not closing

Or should I create a seperate issue for this?

Edit: I am an idiot, you can close it with the keybinding obviously, but it would be nice if it closed by design when you exit an SSH session for example :)

@DHowett
Copy link
Member

DHowett commented Aug 6, 2021

would be nice if it closed by design when you exit an SSH session for example :)

Definitely! That's why we made this configurable with the closeOnExit key (or the profile setting about what to do when the connected application exits! It's in the Advanced tab on the profile page.)

@Don-Vito
Copy link
Contributor

Don-Vito commented Sep 2, 2021

I'm okay with adding something like

to adjust what happens with non-zero exit codes visit Settings->Foo->bar. Read more at <link-to-the-docs....>

But only in the scenario where it's only displayed the first time it happens, because otherwise I feel like it just adds noise. Fortunately, with #8324, we can actually do this. Thanks for the idea!

@zadjii-msft - how do you envision the UX of this. I believe it should be an InfoBar with "Don't show this message again" button rather then printing some text into the terminal.

Probably something like:
image

@ghost ghost added the In-PR This issue has a related PR label Sep 3, 2021
@ghost ghost closed this as completed in #11139 Sep 10, 2021
ghost pushed a commit that referenced this issue Sep 10, 2021
## Summary of the Pull Request
* Introduces info bar shown upon session failure, 
that guides the user how to configure termination behavior
  * Allows this info bar to be dismissed permanently (choice stored in state) 
* Allows "keyboard service" info bar to be dismissed permanently

## PR Checklist
* [x] Closes #10798, #8699
* [x] CLA signed. 
* [ ] Tests added/passed
* [ ] Documentation updated. 
* [ ] Schema updated.
* [ ] I've discussed this with core contributors already.

## Detailed Description of the Pull Request / Additional comments
UI:
* Introduce an additional info bar for "close on exit" configuration tip
  * Stack this bar after "keyboard service" bar
* Add "Don't show again" button to both bars

Dismiss Permanently:
* Introduce a set of "dismissed messages" to the Application State
* Add verification the message is not dismissed before showing an info bar
* "Don't show  again" persists the choice under "dismissed messages"

Wiring the Info Bar:
* Register `TerminalPage` on `TermControl`'s `ConnectionStateChanged` event
* Once event is triggered check whether the state is failure
* If so and the message was not dismissed permanently, show the info bar
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Sep 10, 2021
@ghost
Copy link

ghost commented Oct 20, 2021

🎉This issue was addressed in #11139, which has now been successfully released as Windows Terminal v1.11.2921.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Oct 20, 2021

🎉This issue was addressed in #11139, which has now been successfully released as Windows Terminal Preview v1.12.2922.0.:tada:

Handy links:

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Oct 25, 2021

Today, I got the CloseOnExitInfoBar for the first time. I then pressed Ctrl+Shift+W to close the tab, but the info bar remained there.

Should the info bar be local to the tab or even to the pane? I suppose it shouldn't, because KeyboardServiceWarningInfoBar and SetAsDefaultInfoBar aren't local either, and because the info bar shows up on nonzero exit codes even if Windows Terminal has already been configured to close the tab regardless.

If the info bar is not local to the tab, then its location between the tab bar (when the title bar is hidden) and the terminal panes feels misleading. Should it instead be above the tab bar or below the terminal panes?

How can I close the info bar by using the keyboard?

@zadjii-msft
Copy link
Member

because the info bar shows up on nonzero exit codes even if Windows Terminal has already been configured to close the tab regardless.

well that's not what you want. I'll file that.

If the info bar is not local to the tab, then its location between the tab bar (when the title bar is hidden) and the terminal panes feels misleading. Should it instead be above the tab bar or below the terminal panes?

You're... not wrong here. It's maybe not the best place for it, but on top of the tabs would look insane, and probably wouldn't even work easily, given the way we reparent the tab row into the titlebar.

In the pane itself would make more sense, but that runs abreast of all sorts of things like #9024, #4998, which might make more sense.

I then pressed Ctrl+Shift+W to close the tab, but the info bar remained there.

That, that we could probably fix.

zadjii-msft added a commit that referenced this issue Oct 25, 2021
  Fixes #11606

  This is weird, but the infobars would appear totally on top of the
  TerminalPage when `showTabsInTitlebar:false`. This would result in the infobar
  obscuring the tabs.

  Now, the infobars are strictly inserted after the tabs, before the content. So
  when they appear, they will reduce the amount of space usable for the control.
  That is a little annoying, but preferable to the tabs totally not existing.

  Relevant conversation notes from #10798:

  > > If the info bar is not local to the tab, then its location between the tab
  > > bar (when the title bar is hidden) and the terminal panes feels
  > > misleading. Should it instead be above the tab bar or below the terminal
  > > panes?
  >
  > You're... not wrong here. It's maybe not the best place for it, but _on top_
  > of the tabs would look insane, and probably wouldn't even work easily, given
  > the way we reparent the tab row into the titlebar.
  >
  > In the pane itself would make more sense, but that runs abreast of all sorts
  > of things like #9024, #4998, which might make more sense.

  I'm just gonna go with this now, because it's _better_ than before, while we
  work out what's _best_.
ghost pushed a commit that referenced this issue Oct 26, 2021
…of (#11609)

Fixes #11606

This is weird, but the infobars would appear totally on top of the
TerminalPage when `showTabsInTitlebar:false`. This would result in the infobar
obscuring the tabs.

Now, the infobars are strictly inserted after the tabs, before the content. So
when they appear, they will reduce the amount of space usable for the control.
That is a little annoying, but preferable to the tabs totally not existing.

Relevant conversation notes from #10798:

> > If the info bar is not local to the tab, then its location between the tab
> > bar (when the title bar is hidden) and the terminal panes feels
> > misleading. Should it instead be above the tab bar or below the terminal
> > panes?
>
> You're... not wrong here. It's maybe not the best place for it, but _on top_
> of the tabs would look insane, and probably wouldn't even work easily, given
> the way we reparent the tab row into the titlebar.
>
> In the pane itself would make more sense, but that runs abreast of all sorts
> of things like #9024, #4998, which might make more sense.

I'm just gonna go with this now, because it's _better_ than before, while we
work out what's _best_.

![gh-11606-fix](https://user-images.githubusercontent.com/18356694/138729178-b96b7003-0dd2-4521-8fff-0fd2a5989f22.gif)
DHowett pushed a commit that referenced this issue Dec 13, 2021
…of (#11609)

Fixes #11606

This is weird, but the infobars would appear totally on top of the
TerminalPage when `showTabsInTitlebar:false`. This would result in the infobar
obscuring the tabs.

Now, the infobars are strictly inserted after the tabs, before the content. So
when they appear, they will reduce the amount of space usable for the control.
That is a little annoying, but preferable to the tabs totally not existing.

Relevant conversation notes from #10798:

> > If the info bar is not local to the tab, then its location between the tab
> > bar (when the title bar is hidden) and the terminal panes feels
> > misleading. Should it instead be above the tab bar or below the terminal
> > panes?
>
> You're... not wrong here. It's maybe not the best place for it, but _on top_
> of the tabs would look insane, and probably wouldn't even work easily, given
> the way we reparent the tab row into the titlebar.
>
> In the pane itself would make more sense, but that runs abreast of all sorts
> of things like #9024, #4998, which might make more sense.

I'm just gonna go with this now, because it's _better_ than before, while we
work out what's _best_.

![gh-11606-fix](https://user-images.githubusercontent.com/18356694/138729178-b96b7003-0dd2-4521-8fff-0fd2a5989f22.gif)

(cherry picked from commit a916a5d)
DHowett pushed a commit that referenced this issue Dec 13, 2021
…of (#11609)

Fixes #11606

This is weird, but the infobars would appear totally on top of the
TerminalPage when `showTabsInTitlebar:false`. This would result in the infobar
obscuring the tabs.

Now, the infobars are strictly inserted after the tabs, before the content. So
when they appear, they will reduce the amount of space usable for the control.
That is a little annoying, but preferable to the tabs totally not existing.

Relevant conversation notes from #10798:

> > If the info bar is not local to the tab, then its location between the tab
> > bar (when the title bar is hidden) and the terminal panes feels
> > misleading. Should it instead be above the tab bar or below the terminal
> > panes?
>
> You're... not wrong here. It's maybe not the best place for it, but _on top_
> of the tabs would look insane, and probably wouldn't even work easily, given
> the way we reparent the tab row into the titlebar.
>
> In the pane itself would make more sense, but that runs abreast of all sorts
> of things like #9024, #4998, which might make more sense.

I'm just gonna go with this now, because it's _better_ than before, while we
work out what's _best_.

![gh-11606-fix](https://user-images.githubusercontent.com/18356694/138729178-b96b7003-0dd2-4521-8fff-0fd2a5989f22.gif)

(cherry picked from commit a916a5d)
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-User Interface Issues pertaining to the user interface of the Console or Terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants