Skip to content

Replace confirmCloseAllTabs with confirmOnClose #20055

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.

Sign up for GitHub

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

Open
carlos-zamora wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Replace confirmCloseAllTabs with confirmOnClose #20055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion doc/cascadia/profiles.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2654,10 +2654,21 @@
"type": "string"
},
"warning.confirmCloseAllTabs": {
"deprecated": true,
"description": "[Deprecated] Use \"warning.confirmOnClose\" instead.",
Copy link
Copy Markdown
Member

@lhecker lhecker Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should add a "deprecated": true, property here: https://json-schema.org/understanding-json-schema/reference/annotations

"default": true,
"description": "When set to \"true\" closing a window with multiple tabs open will require confirmation. When set to \"false\", the confirmation dialog will not appear.",
"type": "boolean"
},
"warning.confirmOnClose": {
"default": "automatic",
"description": "Controls when a confirmation dialog appears before closing tabs or windows.",
"enum": [
"never",
"automatic",
"always"
],
"type": "string"
},
"useTabSwitcher": {
"description": "[Deprecated] Replaced with the \"tabSwitcherMode\" setting.",
"default": true,
Expand Down
4 changes: 2 additions & 2 deletions src/cascadia/TerminalApp/AppActionHandlers.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ namespace winrt::TerminalApp::implementation

_RemoveTabs(tabsToRemove);

actionArgs.Handled(true);
actionArgs.Handled(!tabsToRemove.empty());
Copy link
Copy Markdown
Member

@lhecker lhecker Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this just a change on technical correctness or is there a common situation where tabsToRemove ends up being empty?

}
}

Expand Down Expand Up @@ -837,7 +837,7 @@ namespace winrt::TerminalApp::implementation
// tab row, until you mouse over them. Probably has something to do
// with tabs not resizing down until there's a mouse exit event.

actionArgs.Handled(true);
actionArgs.Handled(!tabsToRemove.empty());
}
}

Expand Down
44 changes: 34 additions & 10 deletions src/cascadia/TerminalApp/Resources/en-US/Resources.resw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,24 +496,48 @@
<value>Third-Party notices</value>
<comment>A hyperlink name for the Terminal's third-party notices</comment>
</data>
<data name="QuitDialog.CloseButtonText" xml:space="preserve">
<data name="ConfirmCloseDialog_Cancel" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="QuitDialog.PrimaryButtonText" xml:space="preserve">
<value>Close all</value>
</data>
<data name="QuitDialog.Title" xml:space="preserve">
<data name="ConfirmCloseDialog_CloseAllTitle" xml:space="preserve">
<value>Do you want to close all windows?</value>
</data>
<data name="CloseAllDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
<data name="CloseAllDialog.PrimaryButtonText" xml:space="preserve">
<data name="ConfirmCloseDialog_CloseAllPrimary" xml:space="preserve">
<value>Close all</value>
</data>
<data name="CloseAllDialog.Title" xml:space="preserve">
<data name="ConfirmCloseDialog_WindowTitle" xml:space="preserve">
<value>Do you want to close all tabs?</value>
</data>
<data name="ConfirmCloseDialog_WindowPrimary" xml:space="preserve">
<value>Close all</value>
</data>
<data name="ConfirmCloseDialog_TabTitle" xml:space="preserve">
<value>Do you want to close this tab?</value>
</data>
<data name="ConfirmCloseDialog_TabPrimary" xml:space="preserve">
<value>Close tab</value>
</data>
<data name="ConfirmCloseDialog_PaneTitle" xml:space="preserve">
<value>Do you want to close this pane?</value>
</data>
<data name="ConfirmCloseDialog_PanePrimary" xml:space="preserve">
<value>Close pane</value>
</data>
<data name="ConfirmCloseDialog_MultipleTabsTitle" xml:space="preserve">
<value>Do you want to close these tabs?</value>
</data>
<data name="ConfirmCloseDialog_MultipleTabsPrimary" xml:space="preserve">
<value>Close tabs</value>
</data>
<data name="ConfirmCloseDialog_MultiplePanesTitle" xml:space="preserve">
<value>Do you want to close these panes?</value>
</data>
<data name="ConfirmCloseDialog_MultiplePanesPrimary" xml:space="preserve">
<value>Close panes</value>
</data>
<data name="DontAskAgainCheckBox.Content" xml:space="preserve">
<value>Don't ask me again</value>
</data>
<data name="CloseReadOnlyDialog.CloseButtonText" xml:space="preserve">
<value>Cancel</value>
</data>
Expand Down
85 changes: 79 additions & 6 deletions src/cascadia/TerminalApp/TabManagement.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,9 @@ namespace winrt::TerminalApp::implementation
// - Removes the tab (both TerminalControl and XAML) after prompting for approval
// Arguments:
// - tab: the tab to remove
winrt::Windows::Foundation::IAsyncAction TerminalPage::_HandleCloseTabRequested(winrt::TerminalApp::Tab tab)
// - skipConfirmClose: if true, skip the confirmOnClose check. Used when
// an aggregate confirmation has already been shown (i.e. close other tabs)
winrt::Windows::Foundation::IAsyncAction TerminalPage::_HandleCloseTabRequested(winrt::TerminalApp::Tab tab, bool skipConfirmClose)
{
winrt::com_ptr<TerminalPage> strong;

Expand All @@ -413,6 +415,24 @@ namespace winrt::TerminalApp::implementation
}
}

// Skip the per-tab confirmOnClose check when the caller has already
// shown an aggregate confirmation dialog (e.g. _RemoveTabs).
if (!skipConfirmClose)
{
const auto tabImpl = _GetTabImpl(tab);
if (tabImpl && _ShouldWarnOnCloseTab(tabImpl))
{
const auto weak = get_weak();

auto warningResult = co_await _ShowConfirmCloseDialog(ConfirmCloseDialogKind::Tab);
strong = weak.get();
if (!strong || warningResult != ContentDialogResult::Primary)
{
co_return;
}
}
}

auto t = winrt::get_self<implementation::Tab>(tab);
auto actions = t->BuildStartupActions(BuildStartupKind::None);
_AddPreviouslyClosedPaneOrTab(std::move(actions));
Expand Down Expand Up @@ -782,6 +802,22 @@ namespace winrt::TerminalApp::implementation
if (const auto pane{ activeTab->GetActivePane() })
{
const auto weak = get_weak();

// Check if we should warn before closing a single pane
// (only triggers on Always — Automatic doesn't warn for single pane)
const auto setting = _settings.GlobalSettings().ConfirmOnClose();
if (setting == ConfirmOnClose::Always)
{
// If this is the last pane, closing it closes the tab,
// so use the tab dialog text instead.
const auto kind = activeTab->GetLeafPaneCount() == 1 ? ConfirmCloseDialogKind::Tab : ConfirmCloseDialogKind::Pane;
auto warningResult = co_await _ShowConfirmCloseDialog(kind);
if (!weak.get() || warningResult != ContentDialogResult::Primary)
Copy link
Copy Markdown
Member

@lhecker lhecker Apr 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're immediately dropping the strong reference here and you may be the last holder of this. If you search for !weak.get() in this branch it will come up a few more times.

{
co_return;
}
}

if (co_await _PaneConfirmCloseReadOnly(pane))
{
if (const auto strong = weak.get())
Expand All @@ -795,10 +831,33 @@ namespace winrt::TerminalApp::implementation

// Method Description:
// - Close all panes with the given IDs sequentially.
// - Shows a single aggregate confirmation dialog upfront if the confirmOnClose setting warrants it.
// Arguments:
// - weakTab: weak reference to the tab that the pane belongs to.
// - weakTab: weak reference to the tab that the panes belong to.
// - paneIds: collection of the IDs of the panes that are marked for removal.
void TerminalPage::_ClosePanes(weak_ref<Tab> weakTab, std::vector<uint32_t> paneIds)
safe_void_coroutine TerminalPage::_ClosePanes(weak_ref<Tab> weakTab, std::vector<uint32_t> paneIds)
{
// Show a single aggregate confirmation for closing multiple panes.
if (_settings.GlobalSettings().ConfirmOnClose() != ConfirmOnClose::Never)
{
const auto weak = get_weak();
auto warningResult = co_await _ShowConfirmCloseDialog(ConfirmCloseDialogKind::MultiplePanes);
if (!weak.get() || warningResult != ContentDialogResult::Primary)
{
co_return;
}
}
_CloseRemainingPanes(weakTab, std::move(paneIds));
}

// Method Description:
// - Recursively closes panes by ID, chaining each close via the
// ClosedByParent callback. Called after confirmation has already
// been handled by _ClosePanes.
// Arguments:
// - weakTab: weak reference to the tab that the panes belong to
// - paneIds: remaining pane IDs to close
void TerminalPage::_CloseRemainingPanes(weak_ref<Tab> weakTab, std::vector<uint32_t> paneIds)
{
if (auto strongTab{ weakTab.get() })
{
Expand All @@ -813,10 +872,9 @@ namespace winrt::TerminalApp::implementation
pane->ClosedByParent([ids{ std::move(paneIds) }, weakThis{ get_weak() }, weakTab]() {
if (auto strongThis{ weakThis.get() })
{
strongThis->_ClosePanes(weakTab, std::move(ids));
strongThis->_CloseRemainingPanes(weakTab, std::move(ids));
}
});

// Close the pane which will eventually trigger the closed by parent event
_HandleClosePaneRequested(pane);
break;
Expand All @@ -841,18 +899,33 @@ namespace winrt::TerminalApp::implementation

// Method Description:
// - Closes provided tabs one by one
// - Shows a single aggregate confirmation dialog upfront if the confirmOnClose setting warrants it.
// Arguments:
// - tabs - tabs to remove
safe_void_coroutine TerminalPage::_RemoveTabs(const std::vector<winrt::TerminalApp::Tab> tabs)
{
if (tabs.empty())
{
co_return;
}

// Show a single aggregate confirmation instead of per-tab dialogs.
const auto weak = get_weak();
if (_settings.GlobalSettings().ConfirmOnClose() != ConfirmOnClose::Never)
{
auto warningResult = co_await _ShowConfirmCloseDialog(ConfirmCloseDialogKind::MultipleTabs);
if (!weak.get() || warningResult != ContentDialogResult::Primary)
{
co_return;
}
}

for (auto& tab : tabs)
{
winrt::Windows::Foundation::IAsyncAction action{ nullptr };
if (const auto strong = weak.get())
{
action = _HandleCloseTabRequested(tab);
action = _HandleCloseTabRequested(tab, /*skipConfirmClose*/ true);
}

if (!action)
Expand Down
Loading
Loading