fix(editor): closing a tab no longer throws away unsaved edits

The code panel hangs off the tab, so closing the tab drops every buffer
in it. `editor_close_file` asks before closing a *file*, and the file
tree marks a dirty one — but nothing outside `code_editor` read `dirty`
at all, and `tab_close_reason` looked at `pane.terminals()` and nothing
else. Every close path that went through the tab took the edits without
a word.

The ordinary ⌘W is one of them. `CloseActiveTab` reaches
`editor_close_active_if_focused` only while the editor has focus; with
focus in the terminal — which is where it is after you edit a file and go
back to the shell — it goes to `close_pane`, and a last pane takes its
tab. Type into a file, click the terminal, press ⌘W, and the text is
gone with no dialog.

`UnsavedEdits` is checked first wherever a reason is derived. Of the
three, it is the only loss that cannot be undone by doing the thing
again: a killed command can be re-run and a dropped SSH link
reconnected.

Three paths, because the tab can end from three directions:

- `tab_close_reason`, for a tab closed as a tab.
- `focused_pane_close_reason`, because `close_pane_inner` carries its own
  `confirmed` into `close_tab_inner` so the tab does not ask twice —
  which means a question the pane never asked is never asked at all.
- the bulk closes, which pass `confirmed` and so cannot ask. They skip
  instead, on the same footing as an SSH profile that asked to be warned
  about. Deliberately not the footing of a busy tab: busy is the state
  most tabs on a working window are in, so skipping it would close
  nothing, while unsaved edits are rare and unrecoverable.

Both loss paths were run against the unfixed code and fail there. The
third test — a saved buffer still closing outright — passes either way on
purpose: it holds the guard to the edits rather than to the mere presence
of a code panel.
This commit is contained in:
l0ng-ai
2026-08-22 23:06:58 +08:00
parent 62d44f3d0f
commit b23e0feb55
6 changed files with 200 additions and 7 deletions
+40 -7
View File
@@ -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<CloseReason> {
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<CloseReason> {
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)
}
+152
View File
@@ -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<Self>,
) {
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<String> {
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"
);
});
}
}
+2
View File
@@ -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…",
+2
View File
@@ -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 => "コマンドパレット…",
+2
View File
@@ -95,6 +95,8 @@ l10n_keys! {
NewFolderName,
NewFileName,
HomeNewTab,
CloseUnsavedEditsBody,
CloseUnsavedEditsTitle,
HomeReopenClosedTab,
HomeSwitchWorkspace,
HomeCommandPalette,
+2
View File
@@ -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 => "命令面板…",