From cdf64b6cad090cdac1b1886328883095addb4995 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 17:08:28 -0800 Subject: [PATCH] Unify save and save_as for local worktrees This fixes state propagation bugs due to missing RPC calls in save_as. --- crates/collab/src/tests/integration_tests.rs | 18 ++++-- crates/project/src/project.rs | 6 +- crates/project/src/worktree.rs | 59 ++++++++------------ 3 files changed, 42 insertions(+), 41 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 394dcd808ed74ea6424daaed55c41e90d0973c98..8b922167342c36c8efc69ac8ca36bc5baa383474 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2326,19 +2326,27 @@ async fn test_propagate_saves_and_fs_changes( assert!(buffer.file().is_none()); }); + new_buffer_a.update(cx_a, |buffer, cx| { + buffer.edit([(0..0, "ok")], None, cx); + }); project_a .update(cx_a, |project, cx| { - project.save_buffer_as(new_buffer_a, "/a/file3.rs".into(), cx) + project.save_buffer_as(new_buffer_a.clone(), "/a/file3.rs".into(), cx) }) .await .unwrap(); deterministic.run_until_parked(); - new_buffer_b.read_with(cx_b, |buffer, _| { + new_buffer_b.read_with(cx_b, |buffer_b, _| { assert_eq!( - buffer.file().unwrap().path().as_ref(), + buffer_b.file().unwrap().path().as_ref(), Path::new("file3.rs") ); + + new_buffer_a.read_with(cx_a, |buffer_a, _| { + assert_eq!(buffer_b.saved_mtime(), buffer_a.saved_mtime()); + assert_eq!(buffer_b.saved_version(), buffer_a.saved_version()); + }); }); } @@ -2909,7 +2917,9 @@ async fn test_buffer_conflict_after_save( assert!(!buf.has_conflict()); }); - cx_b.update(|cx| Project::save_buffer(buffer_b.clone(), cx)).await.unwrap(); + cx_b.update(|cx| Project::save_buffer(buffer_b.clone(), cx)) + .await + .unwrap(); cx_a.foreground().forbid_parking(); buffer_b.read_with(cx_b, |buffer_b, _| assert!(!buffer_b.is_dirty())); buffer_b.read_with(cx_b, |buf, _| { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 3cde06e317804ae429b87113bdff2d07500c331a..ba5ca82c02501881f49af77eea7add413ab39d3d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1451,7 +1451,7 @@ impl Project { let worktree = file.worktree.clone(); let path = file.path.clone(); worktree.update(cx, |worktree, cx| match worktree { - Worktree::Local(worktree) => worktree.save_buffer(buffer, path, cx), + Worktree::Local(worktree) => worktree.save_buffer(buffer, path, false, cx), Worktree::Remote(worktree) => worktree.save_buffer(buffer, cx), }) } @@ -1474,7 +1474,9 @@ impl Project { let (worktree, path) = worktree_task.await?; worktree .update(&mut cx, |worktree, cx| match worktree { - Worktree::Local(worktree) => worktree.save_buffer_as(buffer.clone(), path, cx), + Worktree::Local(worktree) => { + worktree.save_buffer(buffer.clone(), path.into(), true, cx) + } Worktree::Remote(_) => panic!("cannot remote buffers as new files"), }) .await?; diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 5edb22461849edc62e216b61f8cb4bf73e1f41e0..9f4446f060f3f8253a9c75d17a548d02a122478b 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -728,8 +728,10 @@ impl LocalWorktree { &self, buffer_handle: ModelHandle, path: Arc, + replace_file: bool, cx: &mut ModelContext, ) -> Task> { + let handle = cx.handle(); let buffer = buffer_handle.read(cx); let rpc = self.client.clone(); @@ -742,53 +744,40 @@ impl LocalWorktree { let save = self.write_file(path, text, buffer.line_ending(), cx); cx.as_mut().spawn(|mut cx| async move { - let mtime = save.await?.mtime; + let entry = save.await?; + if let Some(project_id) = project_id { rpc.send(proto::BufferSaved { project_id, buffer_id, version: serialize_version(&version), - mtime: Some(mtime.into()), + mtime: Some(entry.mtime.into()), fingerprint: serialize_fingerprint(fingerprint), })?; } - buffer_handle.update(&mut cx, |buffer, cx| { - buffer.did_save(version.clone(), fingerprint, mtime, None, cx); - }); - anyhow::Ok((version, fingerprint, mtime)) - }) - } - - pub fn save_buffer_as( - &self, - buffer_handle: ModelHandle, - path: impl Into>, - cx: &mut ModelContext, - ) -> Task> { - let handle = cx.handle(); - let buffer = buffer_handle.read(cx); - - let text = buffer.as_rope().clone(); - let fingerprint = text.fingerprint(); - let version = buffer.version(); - let save = self.write_file(path, text, buffer.line_ending(), cx); - - cx.as_mut().spawn(|mut cx| async move { - let entry = save.await?; - let file = File { - 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| { - buffer.did_save(version, fingerprint, file.mtime, Some(Arc::new(file)), cx); + buffer.did_save( + version.clone(), + fingerprint, + entry.mtime, + if replace_file { + Some(Arc::new(File { + entry_id: entry.id, + worktree: handle, + path: entry.path, + mtime: entry.mtime, + is_local: true, + is_deleted: false, + })) + } else { + None + }, + cx, + ); }); - Ok(()) + Ok((version, fingerprint, entry.mtime)) }) }