mirror of
https://github.com/l0ng-ai/tty7.git
synced 2026-09-21 16:02:20 +00:00
fix(window): ask before a window close throws away an unwritten buffer
Closing a tab has asked since #672, and the reason given there was that unsaved text is the one loss in this product that cannot be undone by doing the thing again. Closing the *window* asked nothing. It reads like the safe exit, and for everything else it is: the shells are the daemon's and keep running, which is exactly why closing a window is the keep-everything exit rather than a quit. But the code panel's buffers are the window's alone, no session file carries them — the session records the layout, not the text — so the window closing is the last moment they exist. Every other guard in the app was pointed at the recoverable losses and this one at nothing. `on_window_should_close` must answer now and a prompt answers later, so the first close is refused and the real one is made from the answer; a flag stops that second close asking again, which would mean a window that never shuts. Only unwritten buffers are asked about — a busy command and a live SSH link survive the window and warning about them would be warning about nothing. Three tests: the decision (which names the tab holding the buffer, not whichever is in front), the flag, and the callback gpui actually calls, driven through `simulate_close` — a guard wired to nothing is no guard, and one wired wrongly is a window that will not close.
This commit is contained in:
+67
-1
@@ -692,6 +692,14 @@ pub struct Tty7App {
|
||||
/// disk, and closing the palette without confirming puts this one back.
|
||||
theme_preview_restore: Option<String>,
|
||||
pub(crate) closed: Vec<SessionTab>,
|
||||
/// Set once the user has answered for the unwritten buffers in this
|
||||
/// window, so the close it then asks for goes straight through.
|
||||
///
|
||||
/// `on_window_should_close` has to answer now and the prompt answers
|
||||
/// later, so the first close is refused and a second one is made from the
|
||||
/// prompt's landing. Without the flag that second close would ask again,
|
||||
/// and the window would never shut.
|
||||
close_confirmed: bool,
|
||||
pub(crate) renaming: Option<Renaming>,
|
||||
pub(crate) worktree_prompt: Option<crate::ui::worktree_prompt::WorktreePrompt>,
|
||||
pub(crate) maximized: Option<Entity<TerminalView>>,
|
||||
@@ -1303,6 +1311,7 @@ impl Tty7App {
|
||||
palette_sub: None,
|
||||
theme_preview_restore: None,
|
||||
closed: Vec::new(),
|
||||
close_confirmed: false,
|
||||
renaming: None,
|
||||
worktree_prompt: None,
|
||||
maximized: None,
|
||||
@@ -1420,7 +1429,17 @@ impl Tty7App {
|
||||
.detach();
|
||||
|
||||
let weak_app = cx.weak_entity();
|
||||
window.on_window_should_close(cx, move |_window, cx| {
|
||||
window.on_window_should_close(cx, move |window, cx| {
|
||||
// Asked before anything is taken down. Closing a window keeps the
|
||||
// shells — they are the daemon's — but the code panel's buffers
|
||||
// are this window's alone and nothing writes them anywhere, so
|
||||
// this is the last moment they exist.
|
||||
if let Some(app) = weak_app.upgrade()
|
||||
&& let Some((tab, name)) = app.read(cx).unsaved_edit_to_confirm()
|
||||
{
|
||||
app.update(cx, |app, cx| app.confirm_window_close(tab, name, window, cx));
|
||||
return false;
|
||||
}
|
||||
let last_window = crate::ui::windows::WindowRegistry::count(cx) <= 1;
|
||||
if let Some(app) = weak_app.upgrade() {
|
||||
app.update(cx, |app, cx| app.detach_workspace(cx));
|
||||
@@ -6203,6 +6222,53 @@ impl Tty7App {
|
||||
})
|
||||
}
|
||||
|
||||
/// What closing the whole window would lose without asking, if anything.
|
||||
///
|
||||
/// Only unwritten buffers. A busy command and a live SSH link are the
|
||||
/// daemon's and survive the window going away — that is the whole point
|
||||
/// of closing being the keep-everything exit — so warning about them here
|
||||
/// would be warning about nothing. Text that was never written down is
|
||||
/// the one thing in the window that has nowhere else to be.
|
||||
pub(crate) fn unsaved_edit_to_confirm(&self) -> Option<(usize, String)> {
|
||||
if self.close_confirmed {
|
||||
return None;
|
||||
}
|
||||
self.unsaved_edit_in_window()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn confirm_window_close_for_test(&mut self) {
|
||||
self.close_confirmed = true;
|
||||
}
|
||||
|
||||
/// Asks about the buffers, then closes the window for real if told to.
|
||||
fn confirm_window_close(
|
||||
&mut self,
|
||||
tab: usize,
|
||||
name: String,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) {
|
||||
// Shown first, so the answer is about a tab the user can see rather
|
||||
// than a file name they have to go looking for.
|
||||
self.activate(tab, window, cx);
|
||||
let answer = window.prompt(
|
||||
PromptLevel::Warning,
|
||||
t(L10nKey::CloseWindowUnsavedEditsTitle),
|
||||
Some(&t_fmt(L10nKey::CloseUnsavedEditsBody, &[("name", &name)])),
|
||||
&crate::ui::confirm_answers(t(L10nKey::Close), t(L10nKey::Keep)),
|
||||
cx,
|
||||
);
|
||||
cx.spawn_in(window, async move |app, cx| {
|
||||
let Ok(0) = answer.await else { return };
|
||||
let _ = app.update_in(cx, |app, window, _cx| {
|
||||
app.close_confirmed = true;
|
||||
window.remove_window();
|
||||
});
|
||||
})
|
||||
.detach();
|
||||
}
|
||||
|
||||
/// Unsaved edits come first, because they are the only loss here that
|
||||
/// cannot be undone by doing the thing again.
|
||||
fn tab_close_reason(&self, index: usize, cx: &App) -> Option<CloseReason> {
|
||||
|
||||
@@ -375,6 +375,20 @@ impl Tty7App {
|
||||
.map(|f| f.label().to_string())
|
||||
}
|
||||
|
||||
/// The first unwritten buffer anywhere in the window, and the tab holding
|
||||
/// it.
|
||||
///
|
||||
/// [`Self::tab_unsaved_edit`] asks about one tab, which is the question a
|
||||
/// tab close has. Closing the *window* takes every tab at once, and had
|
||||
/// been asking nothing at all: the layout is saved and the shells go on
|
||||
/// running under the daemon — that is what makes closing a window the
|
||||
/// keep-everything exit — but a buffer lives in the window, and nothing
|
||||
/// writes it down. So the one loss the tab path calls unrecoverable was
|
||||
/// the one loss the window path took silently.
|
||||
pub(crate) fn unsaved_edit_in_window(&self) -> Option<(usize, String)> {
|
||||
(0..self.tabs.len()).find_map(|ix| Some((ix, self.tab_unsaved_edit(ix)?)))
|
||||
}
|
||||
|
||||
pub(crate) fn tab_code(&self) -> Option<&TabCode> {
|
||||
self.tabs.get(self.active)?.code.as_deref()
|
||||
}
|
||||
@@ -1876,6 +1890,96 @@ mod unsaved_close_gpui_tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// Closing the window asks about a buffer nothing has written down.
|
||||
///
|
||||
/// A tab close has asked since #672; the window close never did. The
|
||||
/// shells survive a window closing — they belong to the daemon, which is
|
||||
/// what makes it the keep-everything exit — but the code panel's buffers
|
||||
/// belong to the window, and no session file carries them. So the one
|
||||
/// loss the tab path calls unrecoverable was the one the window path took
|
||||
/// without a word.
|
||||
#[gpui::test]
|
||||
fn closing_the_window_asks_about_an_unwritten_buffer(cx: &mut TestAppContext) {
|
||||
let (app, mut vcx, _streams) = harness_with_tabs(cx, 3);
|
||||
|
||||
app.update_in(&mut vcx, |app, window, cx| {
|
||||
assert!(
|
||||
app.unsaved_edit_to_confirm().is_none(),
|
||||
"a window with nothing unsaved must close without a question"
|
||||
);
|
||||
|
||||
// Put the buffer in a tab that is *not* the active one: closing
|
||||
// the window takes every tab, so the question cannot be about
|
||||
// whichever one happens to be in front.
|
||||
app.activate(1, window, cx);
|
||||
app.editor_seed_dirty_file_for_test("/w/repo/notes.md", window, cx);
|
||||
app.activate(0, window, cx);
|
||||
|
||||
let (tab, name) = app
|
||||
.unsaved_edit_to_confirm()
|
||||
.expect("the window is holding an unwritten buffer");
|
||||
assert_eq!(tab, 1, "and it names the tab holding it, not the active one");
|
||||
assert_eq!(name, "notes.md");
|
||||
});
|
||||
}
|
||||
|
||||
/// The window really does refuse to close, and really does close when
|
||||
/// there is nothing to lose.
|
||||
///
|
||||
/// The decision is tested above; this drives the callback gpui actually
|
||||
/// calls, because a guard that answers correctly and is wired to nothing
|
||||
/// is the same as no guard — and one wired wrongly is worse: a window
|
||||
/// that will not shut.
|
||||
#[gpui::test]
|
||||
fn the_window_close_callback_refuses_only_when_something_is_unwritten(
|
||||
cx: &mut TestAppContext,
|
||||
) {
|
||||
cx.update(crate::ui::windows::WindowRegistry::init);
|
||||
|
||||
let (app, mut vcx, _streams) = harness_with_tabs(cx, 2);
|
||||
assert!(
|
||||
vcx.simulate_close(),
|
||||
"a window with nothing unsaved closes when asked"
|
||||
);
|
||||
|
||||
let (app2, mut vcx2, _streams2) = harness_with_tabs(cx, 2);
|
||||
app2.update_in(&mut vcx2, |app, window, cx| {
|
||||
app.editor_seed_dirty_file_for_test("/w/repo/notes.md", window, cx);
|
||||
});
|
||||
assert!(
|
||||
!vcx2.simulate_close(),
|
||||
"a window holding an unwritten buffer does not close on the first ask"
|
||||
);
|
||||
|
||||
// And once answered it goes, rather than asking again forever.
|
||||
app2.update(cx, |app, _| app.confirm_window_close_for_test());
|
||||
assert!(
|
||||
vcx2.simulate_close(),
|
||||
"the answer lets the next close through"
|
||||
);
|
||||
let _ = app;
|
||||
}
|
||||
|
||||
/// Once answered, the close that follows is not asked about again.
|
||||
///
|
||||
/// `on_window_should_close` has to answer synchronously and the prompt
|
||||
/// lands later, so the real close is a second one made from the answer.
|
||||
/// Without this the window would ask forever and never shut.
|
||||
#[gpui::test]
|
||||
fn a_confirmed_window_close_is_not_asked_about_twice(cx: &mut TestAppContext) {
|
||||
let (app, mut vcx, _streams) = harness_with_tabs(cx, 2);
|
||||
|
||||
app.update_in(&mut vcx, |app, window, cx| {
|
||||
app.editor_seed_dirty_file_for_test("/w/repo/notes.md", window, cx);
|
||||
assert!(app.unsaved_edit_to_confirm().is_some());
|
||||
app.confirm_window_close_for_test();
|
||||
assert!(
|
||||
app.unsaved_edit_to_confirm().is_none(),
|
||||
"the answer stands for the close it was given for"
|
||||
);
|
||||
});
|
||||
}
|
||||
|
||||
/// Building the buffer the way `editor_install_file` does.
|
||||
fn new_buffer(
|
||||
text: &str,
|
||||
|
||||
@@ -19,6 +19,7 @@ pub fn translate_en(key: L10nKey) -> &'static str {
|
||||
L10nKey::HomeNewTab => "New Tab",
|
||||
L10nKey::CloseUnsavedEditsBody => "{name} has unsaved changes. Closing loses them.",
|
||||
L10nKey::CloseUnsavedEditsTitle => "Close this tab?",
|
||||
L10nKey::CloseWindowUnsavedEditsTitle => "Close this window?",
|
||||
L10nKey::HomeReopenClosedTab => "Reopen Closed Tab",
|
||||
L10nKey::HomeSwitchWorkspace => "Switch Workspace…",
|
||||
L10nKey::HomeCommandPalette => "Command Palette…",
|
||||
|
||||
@@ -19,6 +19,7 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> {
|
||||
L10nKey::HomeNewTab => "新規タブ",
|
||||
L10nKey::CloseUnsavedEditsBody => "{name} に未保存の変更があります。閉じると失われます。",
|
||||
L10nKey::CloseUnsavedEditsTitle => "このタブを閉じますか?",
|
||||
L10nKey::CloseWindowUnsavedEditsTitle => "このウィンドウを閉じますか?",
|
||||
L10nKey::HomeReopenClosedTab => "閉じたタブをもう一度開く",
|
||||
L10nKey::HomeSwitchWorkspace => "ワークスペースを切り替える…",
|
||||
L10nKey::HomeCommandPalette => "コマンドパレット…",
|
||||
|
||||
@@ -97,6 +97,7 @@ l10n_keys! {
|
||||
HomeNewTab,
|
||||
CloseUnsavedEditsBody,
|
||||
CloseUnsavedEditsTitle,
|
||||
CloseWindowUnsavedEditsTitle,
|
||||
HomeReopenClosedTab,
|
||||
HomeSwitchWorkspace,
|
||||
HomeCommandPalette,
|
||||
|
||||
@@ -19,6 +19,7 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> {
|
||||
L10nKey::HomeNewTab => "新标签页",
|
||||
L10nKey::CloseUnsavedEditsBody => "{name} 有未保存的修改,关闭会丢失。",
|
||||
L10nKey::CloseUnsavedEditsTitle => "关闭这个标签页?",
|
||||
L10nKey::CloseWindowUnsavedEditsTitle => "要关闭此窗口吗?",
|
||||
L10nKey::HomeReopenClosedTab => "重新打开已关闭的标签页",
|
||||
L10nKey::HomeSwitchWorkspace => "切换工作区…",
|
||||
L10nKey::HomeCommandPalette => "命令面板…",
|
||||
|
||||
Reference in New Issue
Block a user