Save buffers after restoring hunks in the project diff (#25620)

This PR fixes a bug where using the project diff editor to restore hunks
from a file that's not open in its own buffer would cause those reverts
to be lost once the project diff drops its excerpts for that file.

The fix is to save the buffers after restoring them but before the
excerpts are (potentially) dropped. This is done for the project diff
editor only. If we fail to save the affected files, we add their buffers
to the active workspace, so that the reverted contents are preserved and
the user can try again to save them.

- [x] Get it working
- [x] Test
- [ ] ~~Clean up boolean soup~~

Co-authored-by: Max <max@zed.dev>

Release Notes:

- N/A

Cargo.lock

@@ -5422,6 +5422,7 @@ dependencies = [
  "theme",
  "time",
  "ui",
+ "unindent",
  "util",
  "windows 0.58.0",
  "workspace",

crates/collab/src/tests/integration_tests.rs

@@ -6354,7 +6354,7 @@ async fn test_preview_tabs(cx: &mut TestAppContext) {
     // Open item 1 as preview
     workspace
         .update_in(cx, |workspace, window, cx| {
-            workspace.open_path_preview(path_1.clone(), None, true, true, window, cx)
+            workspace.open_path_preview(path_1.clone(), None, true, true, true, window, cx)
         })
         .await
         .unwrap();
@@ -6375,7 +6375,7 @@ async fn test_preview_tabs(cx: &mut TestAppContext) {
     // Open item 2 as preview
     workspace
         .update_in(cx, |workspace, window, cx| {
-            workspace.open_path_preview(path_2.clone(), None, true, true, window, cx)
+            workspace.open_path_preview(path_2.clone(), None, true, true, true, window, cx)
         })
         .await
         .unwrap();
@@ -6507,7 +6507,7 @@ async fn test_preview_tabs(cx: &mut TestAppContext) {
     // Open item 2 as preview in right pane
     workspace
         .update_in(cx, |workspace, window, cx| {
-            workspace.open_path_preview(path_2.clone(), None, true, true, window, cx)
+            workspace.open_path_preview(path_2.clone(), None, true, true, true, window, cx)
         })
         .await
         .unwrap();
@@ -6545,7 +6545,7 @@ async fn test_preview_tabs(cx: &mut TestAppContext) {
     // Open item 2 as preview in left pane
     workspace
         .update_in(cx, |workspace, window, cx| {
-            workspace.open_path_preview(path_2.clone(), None, true, true, window, cx)
+            workspace.open_path_preview(path_2.clone(), None, true, true, true, window, cx)
         })
         .await
         .unwrap();

crates/editor/src/editor.rs

@@ -7722,7 +7722,7 @@ impl Editor {
         drop(chunk_by);
         if !revert_changes.is_empty() {
             self.transact(window, cx, |editor, window, cx| {
-                editor.revert(revert_changes, window, cx);
+                editor.restore(revert_changes, window, cx);
             });
         }
     }
@@ -15886,13 +15886,16 @@ impl Editor {
         FILE_HEADER_HEIGHT
     }
 
-    pub fn revert(
+    pub fn restore(
         &mut self,
         revert_changes: HashMap<BufferId, Vec<(Range<text::Anchor>, Rope)>>,
         window: &mut Window,
         cx: &mut Context<Self>,
     ) {
-        self.buffer().update(cx, |multi_buffer, cx| {
+        let workspace = self.workspace();
+        let project = self.project.as_ref();
+        let save_tasks = self.buffer().update(cx, |multi_buffer, cx| {
+            let mut tasks = Vec::new();
             for (buffer_id, changes) in revert_changes {
                 if let Some(buffer) = multi_buffer.buffer(buffer_id) {
                     buffer.update(cx, |buffer, cx| {
@@ -15904,9 +15907,44 @@ impl Editor {
                             cx,
                         );
                     });
+
+                    if let Some(project) =
+                        project.filter(|_| multi_buffer.all_diff_hunks_expanded())
+                    {
+                        project.update(cx, |project, cx| {
+                            tasks.push((buffer.clone(), project.save_buffer(buffer, cx)));
+                        })
+                    }
                 }
             }
+            tasks
         });
+        cx.spawn_in(window, |_, mut cx| async move {
+            for (buffer, task) in save_tasks {
+                let result = task.await;
+                if result.is_err() {
+                    let Some(path) = buffer
+                        .read_with(&cx, |buffer, cx| buffer.project_path(cx))
+                        .ok()
+                    else {
+                        continue;
+                    };
+                    if let Some((workspace, path)) = workspace.as_ref().zip(path) {
+                        let Some(task) = cx
+                            .update_window_entity(&workspace, |workspace, window, cx| {
+                                workspace
+                                    .open_path_preview(path, None, false, false, false, window, cx)
+                            })
+                            .ok()
+                        else {
+                            continue;
+                        };
+                        task.await.log_err();
+                    }
+                }
+            }
+        })
+        .detach();
         self.change_selections(None, window, cx, |selections| selections.refresh());
     }
 

crates/file_finder/src/file_finder.rs

@@ -1195,6 +1195,7 @@ impl PickerDelegate for FileFinderDelegate {
                                     None,
                                     true,
                                     allow_preview,
+                                    true,
                                     window,
                                     cx,
                                 )

crates/git_ui/Cargo.toml

@@ -47,6 +47,14 @@ zed_actions.workspace = true
 [target.'cfg(windows)'.dependencies]
 windows.workspace = true
 
+[dev-dependencies]
+editor = { workspace = true, features = ["test-support"] }
+gpui = { workspace = true, features = ["test-support"] }
+project = { workspace = true, features = ["test-support"] }
+settings = { workspace = true, features = ["test-support"] }
+workspace = { workspace = true, features = ["test-support"] }
+unindent.workspace = true
+
 [features]
 default = []
 test-support = ["multi_buffer/test-support"]

crates/git_ui/src/git_panel.rs

@@ -693,7 +693,7 @@ impl GitPanel {
             self.workspace
                 .update(cx, |workspace, cx| {
                     workspace
-                        .open_path_preview(path, None, false, false, window, cx)
+                        .open_path_preview(path, None, false, false, true, window, cx)
                         .detach_and_prompt_err("Failed to open file", window, cx, |e, _, _| {
                             Some(format!("{e}"))
                         });

crates/git_ui/src/project_diff.rs

@@ -47,6 +47,7 @@ pub struct ProjectDiff {
     _subscription: Subscription,
 }
 
+#[derive(Debug)]
 struct DiffBuffer {
     path_key: PathKey,
     buffer: Entity<Buffer>,
@@ -923,3 +924,93 @@ impl Render for ProjectDiffToolbar {
             )
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use collections::HashMap;
+    use editor::test::editor_test_context::assert_state_with_diff;
+    use git::status::{StatusCode, TrackedStatus};
+    use gpui::TestAppContext;
+    use project::FakeFs;
+    use serde_json::json;
+    use settings::SettingsStore;
+    use unindent::Unindent as _;
+    use util::path;
+
+    use super::*;
+
+    fn init_test(cx: &mut TestAppContext) {
+        cx.update(|cx| {
+            let store = SettingsStore::test(cx);
+            cx.set_global(store);
+            theme::init(theme::LoadThemes::JustBase, cx);
+            language::init(cx);
+            Project::init_settings(cx);
+            workspace::init_settings(cx);
+            editor::init(cx);
+            crate::init(cx);
+        });
+    }
+
+    #[gpui::test]
+    async fn test_save_after_restore(cx: &mut TestAppContext) {
+        init_test(cx);
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            path!("/project"),
+            json!({
+                ".git": {},
+                "foo": "FOO\n",
+            }),
+        )
+        .await;
+        let project = Project::test(fs.clone(), [path!("/project").as_ref()], cx).await;
+        let (workspace, cx) =
+            cx.add_window_view(|window, cx| Workspace::test_new(project.clone(), window, cx));
+        let diff = cx.new_window_entity(|window, cx| {
+            ProjectDiff::new(project.clone(), workspace, window, cx)
+        });
+        cx.run_until_parked();
+
+        fs.set_head_for_repo(
+            path!("/project/.git").as_ref(),
+            &[("foo".into(), "foo\n".into())],
+        );
+        fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+            state.statuses = HashMap::from_iter([(
+                "foo".into(),
+                TrackedStatus {
+                    index_status: StatusCode::Unmodified,
+                    worktree_status: StatusCode::Modified,
+                }
+                .into(),
+            )]);
+        });
+        cx.run_until_parked();
+
+        let editor = diff.update(cx, |diff, _| diff.editor.clone());
+        assert_state_with_diff(
+            &editor,
+            cx,
+            &"
+                - foo
+                + FOO
+                  ˇ"
+            .unindent(),
+        );
+
+        editor.update_in(cx, |editor, window, cx| {
+            editor.restore_file(&Default::default(), window, cx);
+        });
+        fs.with_git_state(path!("/project/.git").as_ref(), true, |state| {
+            state.statuses = HashMap::default();
+        });
+        cx.run_until_parked();
+
+        assert_state_with_diff(&editor, cx, &"ˇ".unindent());
+
+        let text = String::from_utf8(fs.read_file_sync("/project/foo").unwrap()).unwrap();
+        assert_eq!(text, "foo\n");
+    }
+}

crates/project_panel/src/project_panel.rs

@@ -485,6 +485,7 @@ impl ProjectPanel {
                                     None,
                                     focus_opened_item,
                                     allow_preview,
+                                    true,
                                     window, cx,
                                 )
                                 .detach_and_prompt_err("Failed to open file", window, cx, move |e, _, _| {

crates/workspace/src/pane.rs

@@ -849,6 +849,7 @@ impl Pane {
         project_entry_id: Option<ProjectEntryId>,
         focus_item: bool,
         allow_preview: bool,
+        activate: bool,
         suggested_position: Option<usize>,
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -876,7 +877,9 @@ impl Pane {
                     }
                 }
             }
-            self.activate_item(index, focus_item, focus_item, window, cx);
+            if activate {
+                self.activate_item(index, focus_item, focus_item, window, cx);
+            }
             existing_item
         } else {
             // If the item is being opened as preview and we have an existing preview tab,
@@ -892,10 +895,11 @@ impl Pane {
             if allow_preview {
                 self.set_preview_item_id(Some(new_item.item_id()), cx);
             }
-            self.add_item(
+            self.add_item_inner(
                 new_item.clone(),
                 true,
                 focus_item,
+                activate,
                 destination_index,
                 window,
                 cx,
@@ -924,11 +928,13 @@ impl Pane {
         }
     }
 
-    pub fn add_item(
+    #[allow(clippy::too_many_arguments)]
+    pub fn add_item_inner(
         &mut self,
         item: Box<dyn ItemHandle>,
         activate_pane: bool,
         focus_item: bool,
+        activate: bool,
         destination_index: Option<usize>,
         window: &mut Window,
         cx: &mut Context<Self>,
@@ -1015,23 +1021,47 @@ impl Pane {
                 cx.notify();
             }
 
-            self.activate_item(insertion_index, activate_pane, focus_item, window, cx);
+            if activate {
+                self.activate_item(insertion_index, activate_pane, focus_item, window, cx);
+            }
         } else {
             self.items.insert(insertion_index, item.clone());
 
-            if insertion_index <= self.active_item_index
-                && self.preview_item_idx() != Some(self.active_item_index)
-            {
-                self.active_item_index += 1;
-            }
+            if activate {
+                if insertion_index <= self.active_item_index
+                    && self.preview_item_idx() != Some(self.active_item_index)
+                {
+                    self.active_item_index += 1;
+                }
 
-            self.activate_item(insertion_index, activate_pane, focus_item, window, cx);
+                self.activate_item(insertion_index, activate_pane, focus_item, window, cx);
+            }
             cx.notify();
         }
 
         cx.emit(Event::AddItem { item });
     }
 
+    pub fn add_item(
+        &mut self,
+        item: Box<dyn ItemHandle>,
+        activate_pane: bool,
+        focus_item: bool,
+        destination_index: Option<usize>,
+        window: &mut Window,
+        cx: &mut Context<Self>,
+    ) {
+        self.add_item_inner(
+            item,
+            activate_pane,
+            focus_item,
+            true,
+            destination_index,
+            window,
+            cx,
+        )
+    }
+
     pub fn items_len(&self) -> usize {
         self.items.len()
     }
@@ -2941,6 +2971,7 @@ impl Pane {
                                                 project_entry_id,
                                                 true,
                                                 false,
+                                                true,
                                                 target,
                                                 window,
                                                 cx,

crates/workspace/src/workspace.rs

@@ -1499,6 +1499,7 @@ impl Workspace {
                                 project_entry_id,
                                 true,
                                 entry.is_preview,
+                                true,
                                 None,
                                 window, cx,
                                 build_item,
@@ -2801,15 +2802,17 @@ impl Workspace {
         window: &mut Window,
         cx: &mut App,
     ) -> Task<Result<Box<dyn ItemHandle>, anyhow::Error>> {
-        self.open_path_preview(path, pane, focus_item, false, window, cx)
+        self.open_path_preview(path, pane, focus_item, false, true, window, cx)
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn open_path_preview(
         &mut self,
         path: impl Into<ProjectPath>,
         pane: Option<WeakEntity<Pane>>,
         focus_item: bool,
         allow_preview: bool,
+        activate: bool,
         window: &mut Window,
         cx: &mut App,
     ) -> Task<Result<Box<dyn ItemHandle>, anyhow::Error>> {
@@ -2830,6 +2833,7 @@ impl Workspace {
                     project_entry_id,
                     focus_item,
                     allow_preview,
+                    activate,
                     None,
                     window,
                     cx,
@@ -2888,6 +2892,7 @@ impl Workspace {
                         project_entry_id,
                         true,
                         allow_preview,
+                        true,
                         None,
                         window,
                         cx,