diff --git a/src/ui/app.rs b/src/ui/app.rs index 85b9fe85..2670edad 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -692,6 +692,14 @@ pub struct Tty7App { /// disk, and closing the palette without confirming puts this one back. theme_preview_restore: Option, pub(crate) closed: Vec, + /// 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, pub(crate) worktree_prompt: Option, pub(crate) maximized: Option>, @@ -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, + ) { + // 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 { diff --git a/src/ui/code_editor.rs b/src/ui/code_editor.rs index 898781eb..3ba6290a 100644 --- a/src/ui/code_editor.rs +++ b/src/ui/code_editor.rs @@ -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, diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 1f2b60ba..ce92dd79 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -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…", diff --git a/src/ui/i18n/ja.rs b/src/ui/i18n/ja.rs index 2bcd9677..b6ba6f84 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -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 => "コマンドパレット…", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index f4e633e3..1ece3564 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -97,6 +97,7 @@ l10n_keys! { HomeNewTab, CloseUnsavedEditsBody, CloseUnsavedEditsTitle, + CloseWindowUnsavedEditsTitle, HomeReopenClosedTab, HomeSwitchWorkspace, HomeCommandPalette, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index c1069cfb..14412c30 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -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 => "命令面板…",