Unify save and save_as for local worktrees

Max Brunsfeld created 2 years ago

This fixes state propagation bugs due to missing RPC calls in save_as.

Change summary

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(-)

Detailed changes

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, _| {

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?;

crates/project/src/worktree.rs

@@ -728,8 +728,10 @@ impl LocalWorktree {
         &self,
         buffer_handle: ModelHandle<Buffer>,
         path: Arc<Path>,
+        replace_file: bool,
         cx: &mut ModelContext<Worktree>,
     ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
+        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<Buffer>,
-        path: impl Into<Arc<Path>>,
-        cx: &mut ModelContext<Worktree>,
-    ) -> Task<Result<()>> {
-        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))
         })
     }