Replace confirmCloseAllTabs with confirmCloseOn

[zadjii-msft]

This PR adds support for a more nuanced enum-backed version of confirmCloseAllTabs.

The old setting was simply a bool true/false, and when set to true, it would warn the user when closing a terminal with more than one pane in it. This lacked fidelity for a variety of scenarios that I enumerated in #6549 (comment).

So instead, this PR changes that setting to a flag-based enum value. We're adding warning.confirmCloseOn, which supports a number of values, which can be combined with one another:

• "multiplePanes" warn if closing any tab with more than one pane
• "multipleTabs" warn if closing a window with more than one tab
• NOT IMPLEMENTED IN THIS PR: "multipleProcesses" warn if closing a pane with more than one client attached
• "always" warn any time a pane/tab/window is about to be closed. Even if there is one pane in one tab with one client, warn.
  • notably, this is different than all (if all === multiplePanes|multipleTabs|multipleProcesses)! That full set doesn't warn on one process in one pane.
• "never" I think you can figure this out

The full behavior of these settings are captured in this spreadsheet:

Settings JSON Before / After

Before	After
{ // Legacy boolean (default: true) "warning.confirmCloseAllTabs": true }	{ // Flags enum — combine any of: // "always", "multipleTabs", // "multiplePanes", "multipleProcesses" "warning.confirmCloseOn": ["multipleTabs", "multiplePanes"] }
{ "warning.confirmCloseAllTabs": false }	{ // Never warn "warning.confirmCloseOn": [] }
N/A — not possible before	{ // Always warn on every close action "warning.confirmCloseOn": ["always"] }
N/A — not possible before	{ // Only warn when closing a tab with // multiple panes (not on window close) "warning.confirmCloseOn": ["multiplePanes"] }

Looping back, these were the originally spec'd scenarios I had called out, and how those settings work after this PR:

#	Scenario	Setting
1	Never be prompted for confirmation	"warning.confirmCloseOn": [] or "warning.confirmCloseOn": ["never"]
2	Prompted when closing window with multiple tabs	"warning.confirmCloseOn": ["multipleTabs"]
3	Prompted when closing a tab with multiple panes (#5301)	"warning.confirmCloseOn": ["multiplePanes"]
4	Prompted when closing a pane with multiple clients1 (#6549)(#5065)(#10784)(countless other dupes)	"warning.confirmCloseOn": ["multipleProcesses"]
5	Prompted when closing a pane, always	"warning.confirmCloseOn": ["always"]
6	(possibly more scenarios)	(other ones haven't come up yet in the last 5 years)
7	Prompted only for multiple clients, not multiple tabs/panes1	"warning.confirmCloseOn": ["multipleProcesses"]
8	Prompted always, even one process / one pane / one tab (#12827)	"warning.confirmCloseOn": ["always"]

• closes closeTab should present a confirmation dialog #5301
• references Prompt before closing pane with multiple client apps #6549
  • we can't close this till we add multipleProcesses
• references Show processes in the "Do you want to close all tabs" dialog #5065
• references [Feature Request] Warn if there is a program running before a tab is closed #10784
• references Settings for notification on window close / system shutdown #12827

Footnotes

1. multipleProcesses isn't implemented yet. Leonard is working on in-proc conpty, which will make that implementation trivial to do correctly. Without that, it is hard to correctly determine the full set of clients attached to a particular console server (without adding additional IPC to conpty) ↩ ↩²

[dsx724]

Can this get reviewed?

[carlos-zamora]

Infrastructure is solid. I'm a bit hesitant on the design for the new setting though. Here's some thoughts:

• Long-term, I think the golden-path should be to have a "don't ask me again" checkbox on the content dialogs (referenced as a checkbox in "Warnings" section here). It's more discoverable and it doesn't require users to go into the settings.
• The current design does have full fidelity with what users want, but I think enum flags are a bit more annoying to deal with for settings (may be personal preference). They're particularly annoying in the UI too (check boxes are more complicated than a toggle button or a dropdown imo).
• Here's my proposed design:
  • replace warning.confirmCloseAllTabs with warning.confirmClose
    • true --> always confirm a close (ignore state.json preferences)
    • false (default) --> don't force confirmations (check state.json preferences)
    • NOTE: if the name's unclear, we can consider warning.forceConfirmClose or warning.alwaysConfirmClose.
  • add "don't ask me again" checkbox to...
    • CloseAllDialog (window w/ multiple tabs)
    • CloseTabDialog (tab w/ multiple panes)
    • ClosePaneDialog (pane w/ any content in it)
  • Since we're writing to the state.json, we don't need to worry about reloading the settings
  • When we can detect multiple processes in a pane, we don't need to update the settings. Just worry about detecting it, displaying a popup, and adding a checkbox.

Let me know what you think. Happy to get this over the finish line too. I just want to converge on a UX first.

[carlos-zamora]

The updated preview doesn't get displayed when you call any of these.

Easy fix though, just add this:
_NotifyChanges(L"ConfirmCloseOnPreview")

[carlos-zamora]

nit: I kinda liked the way you did it earlier in this file more

Suggested change

	if (const auto strong = weak.get())
	{
	if (warningResult != ContentDialogResult::Primary)
	{
	co_return;
	}
	}
	else
	{
	co_return;
	}
	const auto strong = weak.get();
	if (!string || warningResult != ContentDialogResult::Primary)
	{
	co_return;
	}