diff --git a/src/ui/app.rs b/src/ui/app.rs index 3b6281f1..3edfd86b 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -835,6 +835,12 @@ pub(crate) enum CloseTarget { #[derive(Clone, PartialEq, Eq)] pub(crate) enum CloseReason { LiveSsh, + /// A file open in this tab's code panel has edits that were never written. + /// Carries the file's name, because "a file" is not something a reader can + /// act on. Unlike the other two this loss is not recoverable: a killed + /// command can be run again and a dropped SSH link reconnected, but the + /// text is simply gone. + UnsavedEdits(String), Busy(crate::terminal::view::PaneBusy), } @@ -847,6 +853,10 @@ fn close_prompt(ends_the_tab: bool, reason: &CloseReason) -> (String, String) { t(L10nKey::CloseSshConnectionTitle).to_string(), t(L10nKey::CloseSshConnectionBody).to_string(), ), + CloseReason::UnsavedEdits(name) => ( + t(L10nKey::CloseUnsavedEditsTitle).to_string(), + t_fmt(L10nKey::CloseUnsavedEditsBody, &[("name", name)]), + ), CloseReason::Busy(busy) => { let title = match ends_the_tab { true => t(L10nKey::CloseTabBusyTitle), @@ -4305,8 +4315,14 @@ impl Tty7App { // deliberately does *not* skip merely busy tabs: on a working window // that is most of them, and a menu item that quietly closes nothing is // worse than one that closes what it says. + // + // Unsaved edits are skipped for the SSH reason rather than the busy + // one, and the difference is what the skip costs. Busy is the state + // most tabs are in, so skipping it would close nothing; unsaved edits + // are rare, and they are the one loss here that cannot be undone by + // doing the thing again. for i in (0..self.tabs.len()).rev() { - if i == index || self.tab_has_warn_ssh(i, cx) { + if i == index || self.tab_keeps_unclosed_work(i, cx) { continue; } self.close_tab_inner(i, true, window, cx); @@ -4321,7 +4337,7 @@ impl Tty7App { ) { // Same bargain as `close_other_tabs`. for i in ((index + 1)..self.tabs.len()).rev() { - if self.tab_has_warn_ssh(i, cx) { + if self.tab_keeps_unclosed_work(i, cx) { continue; } self.close_tab_inner(i, true, window, cx); @@ -6169,6 +6185,12 @@ impl Tty7App { /// Whether closing this tab would drop a connection the user asked to be /// warned about. Narrower than [`Self::tab_close_reason`] on purpose — see /// the bulk closes, which skip these and only these. + /// What a bulk close leaves standing: the two losses worth refusing to + /// take without being asked, where a busy command is not one. + fn tab_keeps_unclosed_work(&self, index: usize, cx: &App) -> bool { + self.tab_has_warn_ssh(index, cx) || self.tab_unsaved_edit(index).is_some() + } + fn tab_has_warn_ssh(&self, index: usize, cx: &App) -> bool { self.tabs.get(index).is_some_and(|tab| { tab.pane @@ -6178,7 +6200,12 @@ impl Tty7App { }) } + /// 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 { + if let Some(name) = self.tab_unsaved_edit(index) { + return Some(CloseReason::UnsavedEdits(name)); + } self.tabs .get(index)? .pane @@ -6188,11 +6215,17 @@ impl Tty7App { } fn focused_pane_close_reason(&self, window: &Window, cx: &App) -> Option { - let leaf = self - .tabs - .get(self.active)? - .pane - .focused_or_first(window, cx)?; + let tab = self.tabs.get(self.active)?; + // Closing the last pane takes the tab, and the tab's code panel with + // it. `close_pane_inner` carries its own `confirmed` into + // `close_tab_inner` so the tab does not ask a second question, which + // means a question the pane never asked is never asked at all. + if tab.pane.leaves().len() <= 1 + && let Some(name) = self.tab_unsaved_edit(self.active) + { + return Some(CloseReason::UnsavedEdits(name)); + } + let leaf = tab.pane.focused_or_first(window, cx)?; self.leaf_close_reason(&leaf, cx) } diff --git a/src/ui/code_editor.rs b/src/ui/code_editor.rs index c9cff653..57080df0 100644 --- a/src/ui/code_editor.rs +++ b/src/ui/code_editor.rs @@ -276,6 +276,63 @@ fn settle_save(ok: bool, wrote_seq: u64, current_seq: u64, pending: bool) -> Sav } impl Tty7App { + /// Puts a file into the active tab's code panel and marks it dirty, for + /// tests about what closing that tab costs. + /// + /// The real path (`editor_open_on_host`) goes to the filesystem and back + /// through a host round trip; this is the state that arrives at the end of + /// it, which is all the close paths can see. + #[cfg(all(test, unix))] + pub(crate) fn editor_seed_dirty_file_for_test( + &mut self, + path: &str, + window: &mut Window, + cx: &mut Context, + ) { + let host = tty7_core::host::local::LocalHost::new(); + let input = cx.new(|cx| InputState::new(window, cx).multi_line(true)); + let file = OpenFile { + path: PathBuf::from(path), + host, + dirty: true, + disk_mtime: None, + edit_seq: 1, + saving: None, + save_pending: false, + save_then_close: false, + reload_seq: 0, + conflict: false, + preview: false, + wrap: false, + preview_scroll: gpui::ScrollHandle::new(), + _sub: cx.subscribe_in(&input, window, |_, _, _: &InputEvent, _, _| {}), + _observe: cx.observe(&input, |_, _, _| {}), + input, + }; + if let Some(code) = self.tab_code_mut_or_init() { + code.files.push(file); + } + } + + /// The unsaved file a tab would take with it, if it has one. + /// + /// The code panel hangs off the *tab*, so closing the tab drops every + /// buffer in it. The editor asks before closing a file (`editor_close_file`) + /// and the file tree marks a dirty one, but nothing outside this module + /// read `dirty` at all — so closing the tab, which is the ordinary ⌘W when + /// focus is anywhere but the editor, threw the edits away without a word. + /// This is what the close paths ask. + /// + /// The first one, not a count: the question is whether anything would be + /// lost, and a name reads better in a dialog than a number. + pub(crate) fn tab_unsaved_edit(&self, index: usize) -> Option { + let files = &self.tabs.get(index)?.code.as_deref()?.files; + files + .iter() + .find(|f| f.dirty) + .map(|f| f.label().to_string()) + } + pub(crate) fn tab_code(&self) -> Option<&TabCode> { self.tabs.get(self.active)?.code.as_deref() } @@ -1547,3 +1604,98 @@ mod tests { ); } } + +#[cfg(all(test, unix))] +mod unsaved_close_gpui_tests { + use gpui::TestAppContext; + + use crate::ui::app::test_window::harness_with_tabs; + + /// Closing a tab must not throw away edits nobody wrote down. + /// + /// The code panel hangs off the tab, so the tab closing takes every buffer + /// in it. `editor_close_file` asks before closing a *file*, but that is + /// only reached while the editor has focus: the ordinary ⌘W with focus in + /// the terminal goes to `close_pane` and then `close_tab_inner`, and + /// `tab_close_reason` looked at `pane.terminals()` and nothing else. + /// Nothing outside `code_editor` read `dirty` at all, so the edits went + /// without a word. + #[gpui::test] + fn a_tab_with_unsaved_edits_asks_before_it_closes(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_eq!( + app.tab_unsaved_edit(app.active).as_deref(), + Some("notes.md"), + "the tab is holding an unwritten buffer" + ); + app.close_tab(app.active, window, cx); + }); + + app.update(cx, |app, _| { + assert_eq!( + app.tabs.len(), + 2, + "the tab is still here, pending an answer" + ); + }); + } + + /// The same tab with nothing unsaved closes straight away — the guard has + /// to be the edits, not the presence of a code panel. + #[gpui::test] + fn a_tab_whose_buffers_are_saved_still_closes_outright(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); + if let Some(code) = app.tab_code_mut() { + for f in &mut code.files { + f.dirty = false; + } + } + assert!(app.tab_unsaved_edit(app.active).is_none()); + app.close_tab(app.active, window, cx); + }); + + app.update(cx, |app, _| { + assert_eq!(app.tabs.len(), 1, "nothing to ask about") + }); + } + + /// A bulk close spares it instead of asking. + /// + /// One dialog per tab is not a question anyone can answer, so the bargain + /// for "Close Other Tabs" is to skip what it will not take silently. That + /// list holds the SSH profiles that asked to be warned about, and now the + /// unsaved buffers — but deliberately not merely busy tabs, which on a + /// working window is most of them. + #[gpui::test] + fn a_bulk_close_leaves_the_tab_holding_unsaved_edits(cx: &mut TestAppContext) { + let (app, mut vcx, _streams) = harness_with_tabs(cx, 3); + + app.update_in(&mut vcx, |app, window, cx| { + app.activate(1, window, cx); + app.editor_seed_dirty_file_for_test("/w/repo/draft.rs", window, cx); + // Keep tab 0, which would otherwise close tabs 1 and 2. + app.close_other_tabs(0, window, cx); + }); + + app.update(cx, |app, _| { + assert_eq!( + app.tabs.len(), + 2, + "the kept tab and the one still holding an unwritten buffer" + ); + assert!( + app.tabs.iter().any(|t| t + .code + .as_deref() + .is_some_and(|c| c.files.iter().any(|f| f.dirty))), + "and the one spared is the one with the edits" + ); + }); + } +} diff --git a/src/ui/i18n/en.rs b/src/ui/i18n/en.rs index 9a84c3f0..513ccde8 100644 --- a/src/ui/i18n/en.rs +++ b/src/ui/i18n/en.rs @@ -17,6 +17,8 @@ pub fn translate_en(key: L10nKey) -> &'static str { L10nKey::NewFolderName => "New folder name", L10nKey::NewFileName => "New file name", L10nKey::HomeNewTab => "New Tab", + L10nKey::CloseUnsavedEditsBody => "{name} has unsaved changes. Closing loses them.", + L10nKey::CloseUnsavedEditsTitle => "Close this tab?", 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 daebe02e..d8f19204 100644 --- a/src/ui/i18n/ja.rs +++ b/src/ui/i18n/ja.rs @@ -17,6 +17,8 @@ pub fn translate_ja(key: L10nKey) -> Option<&'static str> { L10nKey::NewFolderName => "新しいフォルダ名", L10nKey::NewFileName => "新しいファイル名", L10nKey::HomeNewTab => "新規タブ", + L10nKey::CloseUnsavedEditsBody => "{name} に未保存の変更があります。閉じると失われます。", + L10nKey::CloseUnsavedEditsTitle => "このタブを閉じますか?", L10nKey::HomeReopenClosedTab => "閉じたタブをもう一度開く", L10nKey::HomeSwitchWorkspace => "ワークスペースを切り替える…", L10nKey::HomeCommandPalette => "コマンドパレット…", diff --git a/src/ui/i18n/mod.rs b/src/ui/i18n/mod.rs index bcc88766..4043b548 100644 --- a/src/ui/i18n/mod.rs +++ b/src/ui/i18n/mod.rs @@ -95,6 +95,8 @@ l10n_keys! { NewFolderName, NewFileName, HomeNewTab, + CloseUnsavedEditsBody, + CloseUnsavedEditsTitle, HomeReopenClosedTab, HomeSwitchWorkspace, HomeCommandPalette, diff --git a/src/ui/i18n/zh.rs b/src/ui/i18n/zh.rs index fbf4e6f2..a6ae4832 100644 --- a/src/ui/i18n/zh.rs +++ b/src/ui/i18n/zh.rs @@ -17,6 +17,8 @@ pub fn translate_zh(key: L10nKey) -> Option<&'static str> { L10nKey::NewFolderName => "新文件夹名", L10nKey::NewFileName => "新文件名", L10nKey::HomeNewTab => "新标签页", + L10nKey::CloseUnsavedEditsBody => "{name} 有未保存的修改,关闭会丢失。", + L10nKey::CloseUnsavedEditsTitle => "关闭这个标签页?", L10nKey::HomeReopenClosedTab => "重新打开已关闭的标签页", L10nKey::HomeSwitchWorkspace => "切换工作区…", L10nKey::HomeCommandPalette => "命令面板…",