search

microsoft / terminal

The new Windows Terminal and the original Windows console host, all in the same place!
MIT License
95.77k stars 8.34k forks source link

Prompt before closing pane with multiple client apps #6549

Open vhanla opened 4 years ago

vhanla commented 4 years ago

Description of the new feature/enhancement

Some console apps, like vim, neovim, far, etc, are ignored when you accidentally hit Ctrl+W and your unsaved work is gone forever.

Proposed technical implementation details (optional)

I'm not sure how to do that, but it would be nice to offer an API or something to block closing if some return code is not met.

zadjii-msft commented 4 years ago

You know, I was going to just close this as a combo of #5301, #2976 and #5065. However, none of those cover this specific case, where we prevent closing a terminal tab/pane when there's more than one client in the process tree.

We really don't have a comprehensive discussion of all these requirements anywhere, do we?

Scenarios

  1. I want to never be prompted for confirmation
  2. I want to be prompted for confirmation when I close the window with multiple tabs
  3. I want to be prompted for confirmation when I close a tab with multiple panes (#5301)
  4. I want to be prompted for confirmation when I close a pane with multiple clients (This thread #6549, also #5065 & #10784 are dupes)
  5. I want to be prompted for confirmation when I close a pane always (#12827)
  6. possibly more scenarios?
  7. I want to be prompted for confirmation when I close a pane with multiple clients, but I don't care about closing multiple tabs/panes with a single client in each.

Furthermore, we've only got one setting so far - confirmCloseAllTabs. This basically enables 1 and 2. #5874 enables 3 for everyone, without a way to opt-out.

zadjii-msft commented 4 years ago

Uhg, so the way I see it is something like this image

which is, unwieldy to fill out, and I'm not super passionate about filling in right now. If someone has a better idea, please feel free to chime in.

Chips1234 commented 4 years ago

You know, I was going to just close this as a combo of #5301, #2976 and #5065. However, none of those cover this specific case, where we prevent closing a terminal tab/pane when there's more than one client in the process tree.

We really don't have a comprehensive discussion of all these requirements anywhere, do we?

Scenarios

  1. I want to never be prompted for confirmation

  2. I want to be prompted for confirmation when I close the window with multiple tabs

  3. I want to be prompted for confirmation when I close a tab with multiple panes

  4. I want to be prompted for confirmation when I close a pane with multiple clients

  5. I want to be prompted for confirmation when I close a pane always

  6. possibly more scenarios?

  7. I want to be prompted for confirmation when I close a pane with multiple clients, but I don't care about closing multiple tabs/panes with a single client in each.

Furthermore, we've only got one setting so far - confirmCloseAllTabs. This basically enables 1 and 2. #5874 enables 3 for everyone, without a way to opt-out.

@zadjii-MSFT I think your comment is covered with #2976

zadjii-msft commented 4 years ago

Okay yea, i should have been more specific. We want to enable settings to enable/disable the prompts for each of these scenarios. Just prompting before the user quits, regardless of the content, isn't something that I think most people want.

Maybe it's gotta be a flag enum?

enum ConfirmOnClose
{
  Never = 0x0,
  MultipleProcesses = 0x1,
  MultiplePanes = 0x2,
  MultipleTabs = 0x4,
  Always = 0x8,
}

I'm adding the Always as another flag, for "I want to be prompted always, even if there's only one process in one pane in one tab"

zadjii-msft commented 4 years ago

Alright, I think I filled out actually just half this table, but here's an update image

I'm not sure the "multipleTabs" columns of this table will be informative, but I might do them monday.

Now I need to make sure that the above scenarios are all reflected as actual cells in this grid.

zadjii-msft commented 4 years ago

Alright, I think I've done this as comprehensively as possible: image

We've also got a bunch of scenarios listed above, so to re-iterate:

Scenarios

  1. I want to never be prompted for confirmation
  2. I want to be prompted for confirmation when I close the window with multiple tabs
  3. I want to be prompted for confirmation when I close a tab with multiple panes
  4. I want to be prompted for confirmation when I close a pane with multiple clients
  5. I want to be prompted for confirmation when I close a pane always
  6. possibly more scenarios?
  7. I want to be prompted for confirmation when I close a pane with multiple clients, but I don't care about closing multiple tabs/panes with a single client in each.
  8. I want to be prompted always, even if there's only one process in one pane in one tab

The above chart was made with one key assumption. There are basically three actions - closePane, closeTab, and closeWindow. Each one of these actions fundamentally also performs each of the preceding actions on all of the panes/tabs within the tab/window. If any of the preceding actions wants to warn the user, then the executed action should wait for the warning.

So if the user wants to be warned when they close multiple panes in a single tab, and they try to closeWindow a window which has a single tab with multiple panes, that should wait for a warning to be shown.

In my understanding, scenario 5 is no different than scenario 8.

I also considered another setting structure that was more like:

closePaneWarning: ["never", "multipleProcesses", "always"]
closeTabWarning: ["never", "multiplePanes", "always"]
closeWindowWarning: ["never", "multipleTabs", "always"]

This would be a more expressive set of settings with 27 possible combinations (as opposed to the 9 unique cases above). However, if we stick with the same assumption as above, I'm not sure there are actually 27 unique cases. If any child warning "infects" a parent command, then I hypothesize these 27 cases would simplify down to the same 9 cases.

This raises the question if our original assumption is useful or not. Do users really want to be able to be warned when they closeTab with multiple panes, but not when they closeWindow with a tab with multiple panes (simultaneously)? That doesn't really make sense to me. I'm sure there's some user out there who might want that behavior, but I'd argue that it's confusing enough that most users wouldn't want it and it would lead to more confusion than not.

So, in my opinion, using the ConfirmOnClose: [Never, MultipleProcesses, MultiplePanes, MultipleTabs, Always] values make sense to me, and seem to get us the full level of fidelity that would be expected.

Now it's another matter of

htcfreek commented 3 years ago

@zadjii-msft I read your comment with the szenarios. What technicality means "multiple processes"? Does this means two panes where each of them is running a process (e. g. ssh connection)?

zadjii-msft commented 3 years ago

"multiple processes" means that in any pane, there's more than just the immediately attached child running. So case in point would be launching ssh.exe from your "Command Prompt" profile. In that case, the immediate child is cmd.exe, but there's a child of that cmd.exe (the ssh.exe).

It notably could mean one tab, one pane, but in that pane is multiple processes.

htcfreek commented 3 years ago

@zadjii-msft Isn't this a by design behavior? So if a process is runing in cmd/pwsh the pane always have multiple processes? Then imo the case will be "on running processes/jobs" and not "on multiple processes", which can be confusing.

zadjii-msft commented 3 years ago

Yea? In this case "multiple processes" === "on running processes/jobs".

htcfreek commented 3 years ago

Yea? In this case "multiple processes" === "on running processes/jobs".

@zadjii-msft What do you want to say with the question mark behind Yea?

zadjii-msft commented 3 years ago

... nothing in particular? It's just what my inner monologue said when I read your comment 😄

aminya commented 3 years ago

Since #11335 was set as a duplicate of this, the scope of the issue needs to be broadened. I want an option that enables this warning in all cases.

The default value of the option can depend on a particular situation, but there should be one override setting.

zadjii-msft commented 3 years ago

I want an option that enables this warning in all cases.

So, you want ConfirmOnClose: Always, thanks!

major-sam commented 2 years ago

Is it still active issue?

11335 Maybe simply hide [X] tab button option for some profiles(but keep context menu). Stateless apps will be happy when you'll can't close them by miss click

zadjii-msft commented 2 years ago

Yea, it's active in the sense of "we'd like to get to this soon", we've just had other priorities in front of it. Basically, we need to take my comment above, turn it into a spec, get consensus on that, and then just do the work. The actual warning isn't hard, but making sure there's a set of unambiguous settings for all use cases is 😄

schaefsteven commented 1 year ago

My current workaround for this is to always open two tabs, and that way I can get a confirmation whenever I'm closing a Terminal window. But having the ability to confirm on close when Neovim has unsaved work would be at the top of my wishlist for sure. Has there been any progress on this? Even if the options aren't added in the GUI, if we could just set them in the .json, that would be fantastic.

mpql commented 1 year ago

Yea, it's active in the sense of "we'd like to get to this soon"

Any chance this has been spec'd and / or is slated for kind of soon-ish? 😊

zadjii-msft commented 1 year ago

Unfortunately not. This keeps on treading water just below the cut line, unfortunately. I might be able to sneak the spec work in soon, but certainly won't have the cycles to do the dev work.

mpql commented 1 year ago

Appreciate the update! Any progress is good progress if you are indeed able to sneak that spec work in!

aminya commented 1 year ago

I find this specification too complex. https://github.com/microsoft/terminal/issues/6549#issuecomment-675494526

I believe useful progress can be made by implementing the most critical case which is closing a tab that is in the middle of running some application.

zadjii-msft commented 1 year ago

That's actually probably the hardest to implement, because currently the Terminal doesn't really track the whole process tree of things spawned by the Terminal. We'd have to do that first, and plumb that all through.

Big-picture, when I'm writing specs, I find that it's usually best practice to enumerate all the related scenarios in a single doc. That way, designs for a single part won't conflict with the requirements for other related parts.

Ultimately, I think that design ends up being pretty elegant for every possible scenario that we've been asked for. Just one setting that can cover all these use cases seems like best case scenario:

// Just prompt always
"confirmOnClose": "always",

// Prompt on multiple processes, but not for multiple panes/tabs
"confirmOnClose": "multipleProcesses",

// combine flags
"confirmOnClose": ["multipleProcesses", "multiplePanes", "multipleTabs"]
mpql commented 1 year ago

Does the the terminal "own" the X icons on tabs? Because my own case is mostly just that I accidentally close tabs when I mean to activate them, so if it that button could confirm instead of just killing, that would -- I think -- solve that part without needing to track down all spawned processes, etc.

4wk- commented 8 months ago

This close button without prompt is definitely for me the worst part of Windows terminal. I'm always have multiple pane, and sometime I click to change tabs, and too often I hit the X and it closes the entire split tab... A confirm would have been nice :(

I've use the workaround described in another issue to hide the X icons:

Full example:

    "themes": 
    [
         {
            "name": "dark NO CLOSE BUTTON",
            "tab": 
            {
                "background": "terminalBackground",
                "iconStyle": "default",
                "showCloseButton": "never",
                "unfocusedBackground": "#00000000"
            },
            "tabRow": 
            {
                "background": null,
                "unfocusedBackground": "#333333FF"
            },
            "window": 
            {
                "applicationTheme": "dark",
                "experimental.rainbowFrame": false,
                "frame": null,
                "unfocusedFrame": null,
                "useMica": false
            }
        }
    ]

NB: I can now close tabs with CTRL+W or MiddleMouseButton.

tplunket commented 1 month ago

I just closed a tab that I had active work in as I had switched to another tab and wanted to go back... Most of my navigation is done with the keyboard and for my use case I never want to close a terminal tab with the mouse, never ever ever. I can type Ctrl-D or exit or whatever and actually shut down my shells... I literally never ever want to close the window when the command shell is running.... (unless there's a runaway process maybe but then ProcExp will find that.)

I would be satisfied enough with a "do you want to close" confirmation any time I hit the X but honestly I would prefer to have the X button just gone. I get what the above poster has done to create a theme without the close button but now I'd have to figure out how to clone my current theme... and this is not something that has anything to do with theming. (Why would a rando user want a theme to decide the behavior of the close button?)

In my opinion the above matrix is way overwrought. There are three options for each of two settings and they are disjoint:

Sure, getting more complicated for things like looking at the process tree is all fine and good but for almost every single person commenting above it was, "I was trying to switch tabs and I closed it instead." That is what should be addressed and then worry about the rest. Another way to go would be to "never close a not-foreground tab", and maybe that's easiest... but the accidental tab closures are not limited to background tabs. (Personally, I find any application that allows you to mutate state with the mouse when it is not in focus to be terrifying because inevitably something is changed with a mouse click and you have no way to know what.)

lhecker commented 1 month ago

This can be implemented more easily once Windows Terminal is its own console server. Of course, that doesn't work for ssh, but that can't ever work. Always showing a confirmation message would be in line with how other terminals behave.