Merge pull request #1746 from zed-industries/maintain-buffer-identity-across-renames

Antonio Scandurra created

Preserve buffer identity when underlying entry temporarily disappears

Change summary

crates/project/src/project.rs       | 13 ++++---
crates/project/src/project_tests.rs | 52 +++++++++++++++++++++++++++++++
crates/project/src/worktree.rs      | 23 +++++++++----
crates/rpc/proto/zed.proto          |  3 +
4 files changed, 77 insertions(+), 14 deletions(-)

Detailed changes

crates/project/src/project.rs 🔗

@@ -4298,34 +4298,35 @@ impl Project {
                             return;
                         }
 
-                        let new_file = if let Some(entry) = old_file
-                            .entry_id
-                            .and_then(|entry_id| snapshot.entry_for_id(entry_id))
+                        let new_file = if let Some(entry) = snapshot.entry_for_id(old_file.entry_id)
                         {
                             File {
                                 is_local: true,
-                                entry_id: Some(entry.id),
+                                entry_id: entry.id,
                                 mtime: entry.mtime,
                                 path: entry.path.clone(),
                                 worktree: worktree_handle.clone(),
+                                is_deleted: false,
                             }
                         } else if let Some(entry) =
                             snapshot.entry_for_path(old_file.path().as_ref())
                         {
                             File {
                                 is_local: true,
-                                entry_id: Some(entry.id),
+                                entry_id: entry.id,
                                 mtime: entry.mtime,
                                 path: entry.path.clone(),
                                 worktree: worktree_handle.clone(),
+                                is_deleted: false,
                             }
                         } else {
                             File {
                                 is_local: true,
-                                entry_id: None,
+                                entry_id: old_file.entry_id,
                                 path: old_file.path().clone(),
                                 mtime: old_file.mtime(),
                                 worktree: worktree_handle.clone(),
+                                is_deleted: true,
                             }
                         };
 

crates/project/src/project_tests.rs 🔗

@@ -2260,6 +2260,57 @@ async fn test_rescan_and_remote_updates(
     });
 }
 
+#[gpui::test(iterations = 10)]
+async fn test_buffer_identity_across_renames(
+    deterministic: Arc<Deterministic>,
+    cx: &mut gpui::TestAppContext,
+) {
+    let fs = FakeFs::new(cx.background());
+    fs.insert_tree(
+        "/dir",
+        json!({
+            "a": {
+                "file1": "",
+            }
+        }),
+    )
+    .await;
+
+    let project = Project::test(fs, [Path::new("/dir")], cx).await;
+    let tree = project.read_with(cx, |project, cx| project.worktrees(cx).next().unwrap());
+    let tree_id = tree.read_with(cx, |tree, _| tree.id());
+
+    let id_for_path = |path: &'static str, cx: &gpui::TestAppContext| {
+        project.read_with(cx, |project, cx| {
+            let tree = project.worktrees(cx).next().unwrap();
+            tree.read(cx)
+                .entry_for_path(path)
+                .unwrap_or_else(|| panic!("no entry for path {}", path))
+                .id
+        })
+    };
+
+    let dir_id = id_for_path("a", cx);
+    let file_id = id_for_path("a/file1", cx);
+    let buffer = project
+        .update(cx, |p, cx| p.open_buffer((tree_id, "a/file1"), cx))
+        .await
+        .unwrap();
+    buffer.read_with(cx, |buffer, _| assert!(!buffer.is_dirty()));
+
+    project
+        .update(cx, |project, cx| {
+            project.rename_entry(dir_id, Path::new("b"), cx)
+        })
+        .unwrap()
+        .await
+        .unwrap();
+    deterministic.run_until_parked();
+    assert_eq!(id_for_path("b", cx), dir_id);
+    assert_eq!(id_for_path("b/file1", cx), file_id);
+    buffer.read_with(cx, |buffer, _| assert!(!buffer.is_dirty()));
+}
+
 #[gpui::test]
 async fn test_buffer_deduping(cx: &mut gpui::TestAppContext) {
     let fs = FakeFs::new(cx.background());
@@ -2414,6 +2465,7 @@ async fn test_buffer_is_dirty(cx: &mut gpui::TestAppContext) {
         .await
         .unwrap();
     cx.foreground().run_until_parked();
+    buffer2.read_with(cx, |buffer, _| assert!(buffer.is_dirty()));
     assert_eq!(
         *events.borrow(),
         &[

crates/project/src/worktree.rs 🔗

@@ -688,11 +688,12 @@ impl LocalWorktree {
 
             Ok((
                 File {
-                    entry_id: Some(entry.id),
+                    entry_id: entry.id,
                     worktree: handle,
                     path: entry.path,
                     mtime: entry.mtime,
                     is_local: true,
+                    is_deleted: false,
                 },
                 text,
                 diff_base,
@@ -715,11 +716,12 @@ impl LocalWorktree {
         cx.as_mut().spawn(|mut cx| async move {
             let entry = save.await?;
             let file = File {
-                entry_id: Some(entry.id),
+                entry_id: entry.id,
                 worktree: handle,
                 path: entry.path,
                 mtime: entry.mtime,
                 is_local: true,
+                is_deleted: false,
             };
 
             buffer_handle.update(&mut cx, |buffer, cx| {
@@ -1813,8 +1815,9 @@ pub struct File {
     pub worktree: ModelHandle<Worktree>,
     pub path: Arc<Path>,
     pub mtime: SystemTime,
-    pub(crate) entry_id: Option<ProjectEntryId>,
+    pub(crate) entry_id: ProjectEntryId,
     pub(crate) is_local: bool,
+    pub(crate) is_deleted: bool,
 }
 
 impl language::File for File {
@@ -1852,7 +1855,7 @@ impl language::File for File {
     }
 
     fn is_deleted(&self) -> bool {
-        self.entry_id.is_none()
+        self.is_deleted
     }
 
     fn save(
@@ -1912,9 +1915,10 @@ impl language::File for File {
     fn to_proto(&self) -> rpc::proto::File {
         rpc::proto::File {
             worktree_id: self.worktree.id() as u64,
-            entry_id: self.entry_id.map(|entry_id| entry_id.to_proto()),
+            entry_id: self.entry_id.to_proto(),
             path: self.path.to_string_lossy().into(),
             mtime: Some(self.mtime.into()),
+            is_deleted: self.is_deleted,
         }
     }
 }
@@ -1983,8 +1987,9 @@ impl File {
             worktree,
             path: Path::new(&proto.path).into(),
             mtime: proto.mtime.ok_or_else(|| anyhow!("no timestamp"))?.into(),
-            entry_id: proto.entry_id.map(ProjectEntryId::from_proto),
+            entry_id: ProjectEntryId::from_proto(proto.entry_id),
             is_local: false,
+            is_deleted: proto.is_deleted,
         })
     }
 
@@ -1997,7 +2002,11 @@ impl File {
     }
 
     pub fn project_entry_id(&self, _: &AppContext) -> Option<ProjectEntryId> {
-        self.entry_id
+        if self.is_deleted {
+            None
+        } else {
+            Some(self.entry_id)
+        }
     }
 }
 

crates/rpc/proto/zed.proto 🔗

@@ -868,9 +868,10 @@ message User {
 
 message File {
     uint64 worktree_id = 1;
-    optional uint64 entry_id = 2;
+    uint64 entry_id = 2;
     string path = 3;
     Timestamp mtime = 4;
+    bool is_deleted = 5;
 }
 
 message Entry {