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

Ctrl+D to close a tab when profile has exited with error #12849

Closed
NewtonChutney opened this issue Apr 7, 2022 · 8 comments · Fixed by #14060
Closed

Ctrl+D to close a tab when profile has exited with error #12849

NewtonChutney opened this issue Apr 7, 2022 · 8 comments · Fixed by #14060
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. 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.
Milestone

Comments

@NewtonChutney
Copy link

NewtonChutney commented Apr 7, 2022

Description of the new feature/enhancement

Currently, in a Terminal tab, when a process doesn't exit cleanly, i.e., exits with an error code, the terminal profile can either be set to close anyway, or exit if only there isn't any error [error code 0]..
I would like to see a behaviour wherein the error code is shown, and later on a press of Ctrl+D, the tab closes..

This, would be similar to the behaviour in some Linux terminal apps, wherein a terminal exits to the exit code on Ctrl+D, and on another press of Ctrl+D, it exits.

This implementation would make the process of closing WSL profiles quicker and easier.. wherein users logout with Ctrl+D, and then can close the tab on another Ctrl+D, if the process exited with an error code.

Proposed technical implementation details (optional)

-Enhance the existing "Close only when process exits successfully" behaviour,
-or add a new behaviour option,
-or better yet, add a toggle for closing anyways upon Ctrl+D, which will enable it to work for the "Never close automatically" behaviour too! So that a user who might want to see the exit code 0, can please himself and then close the tab..😂

@NewtonChutney NewtonChutney added the Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. label Apr 7, 2022
@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 Apr 7, 2022
@zadjii-msft
Copy link
Member

I mean, you could always just bind Ctrl+D to closePane if you really wanted to always close it on Ctrl+D. That seems a little aggressive (esp. for things where you might actually want to press Ctrl+D, like in emacs or something).

Other discussions:

@NewtonChutney
Copy link
Author

NewtonChutney commented Apr 7, 2022

Yeah, nope, I don't want to close the pane with Ctrl+D..
I'd like to wait on the exit status and then when that's displayed.. A simple repeat of ctrl+D ought to close the pane..

I only want ctrl+D to close come into effect when the error code is shown after the process has exited.. And there isn't any context/process for WT to pass keypresses into..

For eg, I logout from WSL bash using ctrl+D, and the exit code shows in Windows Terminal, I guess the context is handed over to WT completely now.. So, in this moment, when the exit code is being shown, a ctrl+D ought to close the pane/tab/profile..

@zadjii-msft zadjii-msft added Help Wanted We encourage anyone to jump in on these. Area-Settings Issues related to settings and customizability, for console or terminal Product-Terminal The new Windows Terminal. labels Apr 8, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Apr 8, 2022
@zadjii-msft zadjii-msft added this to the Backlog milestone Apr 8, 2022
@zadjii-msft
Copy link
Member

It's not a bad idea. Just need a simple way of expressing the action in json. We don't really have "conditional" keybindings right now, so it's not possible to say something like

{ 
  "command": { "action": "closePane" }, 
  "keys": "ctrl+d", 
  "when": { "connection.state": "closed" } 
},

Without that, we'd need some other way to express this conditional keybinding.

@DHowett
Copy link
Member

DHowett commented Apr 8, 2022

You know, I actually think this should be feature of ConptyConnection or somewhere around that layer. ^D is a common way to signal a shell to exit, and this is such a narrow case where having it rebindable just doesn't matter to me.

I like that we're thinking about making it extensible/contextual (you know how much I love my hobby horse, contextual key bindings!), but... in this case maybe the easy way is easier.

@DHowett DHowett added Issue-Task It's a feature request, but it doesn't really need a major design. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Issue-Feature Complex enough to require an in depth planning process and actual budgeted, scheduled work. labels Apr 8, 2022
@zadjii-msft zadjii-msft added the good first issue This is a fix that might be easier for someone to do as a first contribution label Apr 9, 2022
@zadjii-msft
Copy link
Member

I'm cool with that. Shouldn't be too hard to add.

@sotteson1
Copy link
Contributor

FYI: I'm starting to work on this feature for the MSFT hackathon. https://hackbox.microsoft.com/project/5766

@NewtonChutney
Copy link
Author

FYI: I'm starting to work on this feature for the MSFT hackathon. https://hackbox.microsoft.com/project/5766

Oooohh! Thank you! 😃

@ghost ghost closed this as completed in #14060 Dec 8, 2022
ghost pushed a commit that referenced this issue Dec 8, 2022
…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
@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 Dec 8, 2022
@ghost
Copy link

ghost commented Jan 24, 2023

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

Handy links:

@zadjii-msft zadjii-msft moved this from Should be written to Done in Terminal Walkthroughs May 22, 2023
@github-project-automation github-project-automation bot moved this to Should be written in Terminal Walkthroughs May 22, 2023
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Settings Issues related to settings and customizability, for console or terminal good first issue This is a fix that might be easier for someone to do as a first contribution Help Wanted We encourage anyone to jump in on these. 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
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants