workspace: Prompt to save dirty buffers when close would orphan them (#55889)
The hot-exit shortcut in `save_all_internal` silently serializes dirty buffers instead of prompting. It assumes the workspace will still be reachable, but that's not true for `ReplaceWindow`, or for `CloseWindow` of an empty workspace on macOS — both detach the workspace and orphan its serialized buffers in the DB with no UI path back to them. Only allow the shortcut when the workspace is actually recoverable (`Quit`, `save_last_workspace`, or has visible worktrees). Otherwise prompt. Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #55726 Release Notes: - Fixed unsaved untitled buffers being silently lost when opening a file or project from an empty window
This commit is contained in:
1 parent
7dc02263e7
commit
00d8d45685
2 files changed
+201
-19
No files matched your search
@@ -121,7 +121,7 @@ impl Workspace {
|
||||
let save_action = match save_strategy {
|
||||
SaveStrategy::All => {
|
||||
let save_all = workspace.update_in(cx, |workspace, window, cx| {
|
||||
let task = workspace.save_all_internal(SaveIntent::SaveAll, window, cx);
|
||||
let task = workspace.save_all_internal(SaveIntent::SaveAll, true, window, cx);
|
||||
cx.background_spawn(async { task.await.map(|_| ()) })
|
||||
});
|
||||
save_all.ok()
|
||||
|
||||
@@ -3305,9 +3305,30 @@ impl Workspace {
|
||||
}
|
||||
}
|
||||
|
||||
// Hot-exit silently writes dirty buffers to the DB; only allow it
|
||||
// if the workspace will be reachable again, either via session
|
||||
// restore or by reopening its folder paths. Otherwise prompt, so
|
||||
// we don't orphan the buffers.
|
||||
let allow_hot_exit_serialization = close_intent == CloseIntent::Quit
|
||||
|| save_last_workspace
|
||||
|| this
|
||||
.read_with(cx, |workspace, cx| {
|
||||
workspace
|
||||
.project
|
||||
.read(cx)
|
||||
.visible_worktrees(cx)
|
||||
.next()
|
||||
.is_some()
|
||||
})
|
||||
.unwrap_or(false);
|
||||
let save_result = this
|
||||
.update_in(cx, |this, window, cx| {
|
||||
this.save_all_internal(SaveIntent::Close, window, cx)
|
||||
this.save_all_internal(
|
||||
SaveIntent::Close,
|
||||
allow_hot_exit_serialization,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
})?
|
||||
.await;
|
||||
|
||||
@@ -3328,6 +3349,7 @@ impl Workspace {
|
||||
fn save_all(&mut self, action: &SaveAll, window: &mut Window, cx: &mut Context<Self>) {
|
||||
self.save_all_internal(
|
||||
action.save_intent.unwrap_or(SaveIntent::SaveAll),
|
||||
true,
|
||||
window,
|
||||
cx,
|
||||
)
|
||||
@@ -3425,12 +3447,13 @@ impl Workspace {
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Task<Result<bool>> {
|
||||
self.save_all_internal(SaveIntent::Close, window, cx)
|
||||
self.save_all_internal(SaveIntent::Close, true, window, cx)
|
||||
}
|
||||
|
||||
fn save_all_internal(
|
||||
&mut self,
|
||||
mut save_intent: SaveIntent,
|
||||
allow_hot_exit_serialization: bool,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Self>,
|
||||
) -> Task<Result<bool>> {
|
||||
@@ -3457,23 +3480,27 @@ impl Workspace {
|
||||
let dirty_items = if save_intent == SaveIntent::Close && !dirty_items.is_empty() {
|
||||
let mut serialize_tasks = Vec::new();
|
||||
let mut remaining_dirty_items = Vec::new();
|
||||
workspace.update_in(cx, |workspace, window, cx| {
|
||||
for (pane, item) in dirty_items {
|
||||
if let Some(task) = item
|
||||
.to_serializable_item_handle(cx)
|
||||
.and_then(|handle| handle.serialize(workspace, true, window, cx))
|
||||
{
|
||||
serialize_tasks.push((pane, item, task));
|
||||
} else {
|
||||
if allow_hot_exit_serialization {
|
||||
workspace.update_in(cx, |workspace, window, cx| {
|
||||
for (pane, item) in dirty_items {
|
||||
if let Some(task) = item
|
||||
.to_serializable_item_handle(cx)
|
||||
.and_then(|handle| handle.serialize(workspace, true, window, cx))
|
||||
{
|
||||
serialize_tasks.push((pane, item, task));
|
||||
} else {
|
||||
remaining_dirty_items.push((pane, item));
|
||||
}
|
||||
}
|
||||
})?;
|
||||
|
||||
for (pane, item, task) in serialize_tasks {
|
||||
if task.await.log_err().is_none() {
|
||||
remaining_dirty_items.push((pane, item));
|
||||
}
|
||||
}
|
||||
})?;
|
||||
|
||||
for (pane, item, task) in serialize_tasks {
|
||||
if task.await.log_err().is_none() {
|
||||
remaining_dirty_items.push((pane, item));
|
||||
}
|
||||
} else {
|
||||
remaining_dirty_items = dirty_items;
|
||||
}
|
||||
|
||||
if !remaining_dirty_items.is_empty() {
|
||||
@@ -11473,7 +11500,7 @@ mod tests {
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_close_window_with_serializable_items(cx: &mut TestAppContext) {
|
||||
async fn test_close_window_with_worktrees_hot_exits(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
// Register TestItem as a serializable item
|
||||
@@ -11510,8 +11537,163 @@ mod tests {
|
||||
assert!(task.await.unwrap());
|
||||
}
|
||||
|
||||
// See https://github.com/zed-industries/zed/issues/55726.
|
||||
//
|
||||
// macOS only: on Linux/Windows, closing the last window sets
|
||||
// `save_last_workspace`, which preserves the session (same as `Quit`),
|
||||
// so hot-exit is safe there.
|
||||
#[cfg(target_os = "macos")]
|
||||
#[gpui::test]
|
||||
async fn test_close_window_with_failing_serialization(cx: &mut TestAppContext) {
|
||||
async fn test_close_window_without_worktrees_prompts(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
cx.update(|cx| {
|
||||
register_serializable_item::<TestItem>(cx);
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
let project = Project::test(fs, None, cx).await;
|
||||
let (workspace, cx) =
|
||||
cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
|
||||
let item = cx.new(|cx| {
|
||||
TestItem::new(cx)
|
||||
.with_dirty(true)
|
||||
.with_serialize(|| Some(Task::ready(Ok(()))))
|
||||
});
|
||||
workspace.update_in(cx, |w, window, cx| {
|
||||
w.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx);
|
||||
});
|
||||
|
||||
let task = workspace.update_in(cx, |w, window, cx| {
|
||||
w.prepare_to_close(CloseIntent::CloseWindow, window, cx)
|
||||
});
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
assert!(
|
||||
cx.has_pending_prompt(),
|
||||
"closing a no-folder workspace with a dirty serializable item should prompt, \
|
||||
since the workspace will not be reachable after close"
|
||||
);
|
||||
cx.simulate_prompt_answer("Don't Save");
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
assert!(task.await.unwrap());
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_quit_without_worktrees_hot_exits(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
cx.update(|cx| {
|
||||
register_serializable_item::<TestItem>(cx);
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
let project = Project::test(fs, None, cx).await;
|
||||
let (workspace, cx) =
|
||||
cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
|
||||
let item = cx.new(|cx| {
|
||||
TestItem::new(cx)
|
||||
.with_dirty(true)
|
||||
.with_serialize(|| Some(Task::ready(Ok(()))))
|
||||
});
|
||||
workspace.update_in(cx, |w, window, cx| {
|
||||
w.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx);
|
||||
});
|
||||
|
||||
let task = workspace.update_in(cx, |w, window, cx| {
|
||||
w.prepare_to_close(CloseIntent::Quit, window, cx)
|
||||
});
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
assert!(
|
||||
!cx.has_pending_prompt(),
|
||||
"quitting should hot-exit silently; the session restore on next \
|
||||
launch will bring the dirty buffer back"
|
||||
);
|
||||
assert!(task.await.unwrap());
|
||||
}
|
||||
|
||||
// See https://github.com/zed-industries/zed/issues/55726.
|
||||
#[gpui::test]
|
||||
async fn test_replace_window_without_worktrees_prompts(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
cx.update(|cx| {
|
||||
register_serializable_item::<TestItem>(cx);
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
let project = Project::test(fs, None, cx).await;
|
||||
let (workspace, cx) =
|
||||
cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
|
||||
let item = cx.new(|cx| {
|
||||
TestItem::new(cx)
|
||||
.with_dirty(true)
|
||||
.with_serialize(|| Some(Task::ready(Ok(()))))
|
||||
});
|
||||
workspace.update_in(cx, |w, window, cx| {
|
||||
w.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx);
|
||||
});
|
||||
|
||||
let task = workspace.update_in(cx, |w, window, cx| {
|
||||
w.prepare_to_close(CloseIntent::ReplaceWindow, window, cx)
|
||||
});
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
assert!(
|
||||
cx.has_pending_prompt(),
|
||||
"replacing a workspace with a dirty serializable item should prompt, \
|
||||
since the workspace will be detached afterwards"
|
||||
);
|
||||
cx.simulate_prompt_answer("Don't Save");
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
assert!(task.await.unwrap());
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_replace_window_with_worktrees_hot_exits(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
cx.update(|cx| {
|
||||
register_serializable_item::<TestItem>(cx);
|
||||
});
|
||||
|
||||
let fs = FakeFs::new(cx.executor());
|
||||
fs.insert_tree("/root", json!({ "one": "" })).await;
|
||||
|
||||
let project = Project::test(fs, ["root".as_ref()], cx).await;
|
||||
let (workspace, cx) =
|
||||
cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
|
||||
|
||||
let item = cx.new(|cx| {
|
||||
TestItem::new(cx)
|
||||
.with_dirty(true)
|
||||
.with_serialize(|| Some(Task::ready(Ok(()))))
|
||||
});
|
||||
workspace.update_in(cx, |w, window, cx| {
|
||||
w.add_item_to_active_pane(Box::new(item.clone()), None, true, window, cx);
|
||||
});
|
||||
|
||||
let task = workspace.update_in(cx, |w, window, cx| {
|
||||
w.prepare_to_close(CloseIntent::ReplaceWindow, window, cx)
|
||||
});
|
||||
cx.executor().run_until_parked();
|
||||
|
||||
assert!(
|
||||
!cx.has_pending_prompt(),
|
||||
"replacing a workspace with folder paths should hot-exit silently; \
|
||||
the buffer is recoverable by reopening the project"
|
||||
);
|
||||
assert!(task.await.unwrap());
|
||||
}
|
||||
|
||||
#[gpui::test]
|
||||
async fn test_close_window_with_failing_serialize_prompts(cx: &mut TestAppContext) {
|
||||
init_test(cx);
|
||||
|
||||
cx.update(|cx| {
|
||||
|
||||
Reference in new issue
Block a user