From 0a1aea6cb814810341a2b97ec6d48e7bebd9ac18 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Wed, 12 Oct 2022 17:22:44 +0200 Subject: [PATCH 1/3] Add test to ensure buffer identity is kept across `Project::rename` --- crates/project/src/project_tests.rs | 43 +++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index a631051c58..702f28fc59 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2260,6 +2260,49 @@ async fn test_rescan_and_remote_updates( }); } +#[gpui::test(iterations = 10)] +async fn test_buffer_identity_across_renames(cx: &mut gpui::TestAppContext) { + let dir = temp_tree(json!({ + "a": { + "file1": "", + } + })); + + let project = Project::test(Arc::new(RealFs), [dir.path()], 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(); + tree.flush_fs_events(cx).await; + 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()); From f28cc5ca0c7224f1fed578c2a081c1579edb795a Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 09:10:10 +0200 Subject: [PATCH 2/3] Preserve buffer identity when underlying entry temporarily disappears --- crates/project/src/project.rs | 13 +++++++------ crates/project/src/project_tests.rs | 1 + crates/project/src/worktree.rs | 23 ++++++++++++++++------- crates/rpc/proto/zed.proto | 3 ++- 4 files changed, 26 insertions(+), 14 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 17e0d5fafe..ea74dda434 100644 --- a/crates/project/src/project.rs +++ b/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, } }; diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 702f28fc59..72f4778a38 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2457,6 +2457,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(), &[ diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index f0e9aa7934..383c9ac35b 100644 --- a/crates/project/src/worktree.rs +++ b/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, pub path: Arc, pub mtime: SystemTime, - pub(crate) entry_id: Option, + 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 { - self.entry_id + if self.is_deleted { + None + } else { + Some(self.entry_id) + } } } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 283b11fd78..1248bb0551 100644 --- a/crates/rpc/proto/zed.proto +++ b/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 { From 37a0fd33c5189153d862704ee160768287616601 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Thu, 13 Oct 2022 09:33:55 +0200 Subject: [PATCH 3/3] Use fake file system for buffer identity test --- crates/project/src/project_tests.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 72f4778a38..1b0294c4d1 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2261,14 +2261,22 @@ async fn test_rescan_and_remote_updates( } #[gpui::test(iterations = 10)] -async fn test_buffer_identity_across_renames(cx: &mut gpui::TestAppContext) { - let dir = temp_tree(json!({ - "a": { - "file1": "", - } - })); +async fn test_buffer_identity_across_renames( + deterministic: Arc, + cx: &mut gpui::TestAppContext, +) { + let fs = FakeFs::new(cx.background()); + fs.insert_tree( + "/dir", + json!({ + "a": { + "file1": "", + } + }), + ) + .await; - let project = Project::test(Arc::new(RealFs), [dir.path()], cx).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()); @@ -2297,7 +2305,7 @@ async fn test_buffer_identity_across_renames(cx: &mut gpui::TestAppContext) { .unwrap() .await .unwrap(); - tree.flush_fs_events(cx).await; + 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()));