Move the save and save_as code paths close together

crates/collab/src/tests/integration_tests.rs

@@ -2244,7 +2244,7 @@ async fn test_propagate_saves_and_fs_changes(
     });
 
     // Edit the buffer as the host and concurrently save as guest B.
-    let save_b = buffer_b.update(cx_b, |buf, cx| buf.save(cx));
+    let save_b = cx_b.update(|cx| Project::save_buffer(buffer_b.clone(), cx));
     buffer_a.update(cx_a, |buf, cx| buf.edit([(0..0, "hi-a, ")], None, cx));
     save_b.await.unwrap();
     assert_eq!(
@@ -2909,7 +2909,7 @@ async fn test_buffer_conflict_after_save(
         assert!(!buf.has_conflict());
     });
 
-    buffer_b.update(cx_b, |buf, cx| buf.save(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/collab/src/tests/randomized_integration_tests.rs

@@ -1064,15 +1064,16 @@ async fn randomly_query_and_mutate_buffers(
             }
         }
         30..=39 if buffer.read_with(cx, |buffer, _| buffer.is_dirty()) => {
-            let (requested_version, save) = buffer.update(cx, |buffer, cx| {
+            let requested_version = buffer.update(cx, |buffer, cx| {
                 log::info!(
                     "{}: saving buffer {} ({:?})",
                     client.username,
                     buffer.remote_id(),
                     buffer.file().unwrap().full_path(cx)
                 );
-                (buffer.version(), buffer.save(cx))
+                buffer.version()
             });
+            let save = cx.update(|cx| Project::save_buffer(buffer, cx));
             let save = cx.background().spawn(async move {
                 let (saved_version, _, _) = save
                     .await

crates/editor/src/items.rs

@@ -608,20 +608,11 @@ impl Item for Editor {
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<()>> {
         self.report_event("save editor", cx);
-
+        let format = self.perform_format(project.clone(), cx);
-        let buffer = self.buffer().clone();
+        let buffers = self.buffer().clone().read(cx).all_buffers();
-        let buffers = buffer.read(cx).all_buffers();
-        let save = project.update(cx, |project, cx| project.save_buffers(buffers, cx));
         cx.spawn(|_, mut cx| async move {
-            let (format_transaction, save) = save.await;
+            format.await?;
-            buffer.update(&mut cx, |buffer, _| {
+            cx.update(|cx| Project::save_buffers(buffers, cx)).await?;
-                if let Some(transaction) = format_transaction {
-                    if !buffer.is_singleton() {
-                        buffer.push_transaction(&transaction.0);
-                    }
-                }
-            });
-            save.await?;
             Ok(())
         })
     }
@@ -1144,7 +1135,6 @@ fn path_for_file<'a>(
 mod tests {
     use super::*;
     use gpui::MutableAppContext;
-    use language::RopeFingerprint;
     use std::{
         path::{Path, PathBuf},
         sync::Arc,
@@ -1190,17 +1180,6 @@ mod tests {
             todo!()
         }
 
-        fn save(
-            &self,
-            _: u64,
-            _: language::Rope,
-            _: clock::Global,
-            _: project::LineEnding,
-            _: &mut MutableAppContext,
-        ) -> gpui::Task<anyhow::Result<(clock::Global, RopeFingerprint, SystemTime)>> {
-            todo!()
-        }
-
         fn as_any(&self) -> &dyn std::any::Any {
             todo!()
         }

crates/language/src/buffer.rs

@@ -214,15 +214,6 @@ pub trait File: Send + Sync {
 
     fn is_deleted(&self) -> bool;
 
-    fn save(
-        &self,
-        buffer_id: u64,
-        text: Rope,
-        version: clock::Global,
-        line_ending: LineEnding,
-        cx: &mut MutableAppContext,
-    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>>;
-
     fn as_any(&self) -> &dyn Any;
 
     fn to_proto(&self) -> rpc::proto::File;
@@ -529,33 +520,6 @@ impl Buffer {
         self.file.as_ref()
     }
 
-    pub fn save(
-        &mut self,
-        cx: &mut ModelContext<Self>,
-    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
-        let file = if let Some(file) = self.file.as_ref() {
-            file
-        } else {
-            return Task::ready(Err(anyhow!("buffer has no file")));
-        };
-        let text = self.as_rope().clone();
-        let version = self.version();
-        let save = file.save(
-            self.remote_id(),
-            text,
-            version,
-            self.line_ending(),
-            cx.as_mut(),
-        );
-        cx.spawn(|this, mut cx| async move {
-            let (version, fingerprint, mtime) = save.await?;
-            this.update(&mut cx, |this, cx| {
-                this.did_save(version.clone(), fingerprint, mtime, None, cx);
-            });
-            Ok((version, fingerprint, mtime))
-        })
-    }
-
     pub fn saved_version(&self) -> &clock::Global {
         &self.saved_version
     }

crates/project/src/project.rs

@@ -28,8 +28,8 @@ use language::{
     range_from_lsp, range_to_lsp, Anchor, Bias, Buffer, CachedLspAdapter, CharKind, CodeAction,
     CodeLabel, Completion, Diagnostic, DiagnosticEntry, DiagnosticSet, Event as BufferEvent,
     File as _, Language, LanguageRegistry, LanguageServerName, LocalFile, OffsetRangeExt,
-    Operation, Patch, PointUtf16, TextBufferSnapshot, ToOffset, ToPointUtf16, Transaction,
+    Operation, Patch, PointUtf16, RopeFingerprint, TextBufferSnapshot, ToOffset, ToPointUtf16,
-    Unclipped,
+    Transaction, Unclipped,
 };
 use lsp::{
     DiagnosticSeverity, DiagnosticTag, DocumentHighlightKind, LanguageServer, LanguageString,
@@ -59,7 +59,7 @@ use std::{
         atomic::{AtomicUsize, Ordering::SeqCst},
         Arc,
     },
-    time::{Duration, Instant},
+    time::{Duration, Instant, SystemTime},
 };
 use terminal::{Terminal, TerminalBuilder};
 use util::{debug_panic, defer, post_inc, ResultExt, TryFutureExt as _};
@@ -1429,33 +1429,30 @@ impl Project {
     }
 
     pub fn save_buffers(
-        &mut self,
         buffers: HashSet<ModelHandle<Buffer>>,
-        cx: &mut ModelContext<Self>,
+        cx: &mut MutableAppContext,
-    ) -> Task<(Option<ProjectTransaction>, Task<Result<()>>)> {
+    ) -> Task<Result<()>> {
-        const FORMAT_TIMEOUT: Duration = Duration::from_secs(2);
+        cx.spawn(|mut cx| async move {
+            let save_tasks = buffers
+                .into_iter()
+                .map(|buffer| cx.update(|cx| Self::save_buffer(buffer, cx)));
+            try_join_all(save_tasks).await?;
+            Ok(())
+        })
+    }
 
-        let mut timeout = cx.background().timer(FORMAT_TIMEOUT).fuse();
+    pub fn save_buffer(
-        let format = self.format(buffers.clone(), true, FormatTrigger::Save, cx);
+        buffer: ModelHandle<Buffer>,
-        cx.spawn(|_, cx| async move {
+        cx: &mut MutableAppContext,
-            let transaction = futures::select_biased! {
+    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
-                _ = timeout => {
+        let Some(file) = File::from_dyn(buffer.read(cx).file()) else {
-                    log::warn!("timed out waiting for formatting");
+            return Task::ready(Err(anyhow!("buffer doesn't have a file")));
-                    None
+        };
-                }
+        let worktree = file.worktree.clone();
-                transaction = format.log_err().fuse() => transaction,
+        let path = file.path.clone();
-            };
+        worktree.update(cx, |worktree, cx| match worktree {
-
+            Worktree::Local(worktree) => worktree.save_buffer(buffer, path, cx),
-            (
+            Worktree::Remote(worktree) => worktree.save_buffer(buffer, cx),
-                transaction,
-                cx.spawn(|mut cx| async move {
-                    let save_tasks = buffers
-                        .iter()
-                        .map(|buffer| buffer.update(&mut cx, |buffer, cx| buffer.save(cx)));
-                    try_join_all(save_tasks).await?;
-                    Ok(())
-                }),
-            )
         })
     }
 
@@ -1476,11 +1473,9 @@ impl Project {
             }
             let (worktree, path) = worktree_task.await?;
             worktree
-                .update(&mut cx, |worktree, cx| {
+                .update(&mut cx, |worktree, cx| match worktree {
-                    worktree
+                    Worktree::Local(worktree) => worktree.save_buffer_as(buffer.clone(), path, cx),
-                        .as_local_mut()
+                    Worktree::Remote(_) => panic!("cannot remote buffers as new files"),
-                        .unwrap()
-                        .save_buffer_as(buffer.clone(), path, cx)
                 })
                 .await?;
             this.update(&mut cx, |this, cx| {
@@ -5187,7 +5182,7 @@ impl Project {
             .await;
 
         let (saved_version, fingerprint, mtime) =
-            buffer.update(&mut cx, |buffer, cx| buffer.save(cx)).await?;
+            cx.update(|cx| Self::save_buffer(buffer, cx)).await?;
         Ok(proto::BufferSaved {
             project_id,
             buffer_id,

crates/project/src/project_tests.rs

@@ -243,8 +243,7 @@ async fn test_managing_language_servers(
     );
 
     // Save notifications are reported to all servers.
-    toml_buffer
+    cx.update(|cx| Project::save_buffer(toml_buffer, cx))
-        .update(cx, |buffer, cx| buffer.save(cx))
         .await
         .unwrap();
     assert_eq!(
@@ -2083,12 +2082,12 @@ async fn test_save_file(cx: &mut gpui::TestAppContext) {
         .update(cx, |p, cx| p.open_local_buffer("/dir/file1", cx))
         .await
         .unwrap();
-    buffer
+    buffer.update(cx, |buffer, cx| {
-        .update(cx, |buffer, cx| {
+        assert_eq!(buffer.text(), "the old contents");
-            assert_eq!(buffer.text(), "the old contents");
+        buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx);
-            buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx);
+    });
-            buffer.save(cx)
+
-        })
+    cx.update(|cx| Project::save_buffer(buffer.clone(), cx))
         .await
         .unwrap();
 
@@ -2112,11 +2111,11 @@ async fn test_save_in_single_file_worktree(cx: &mut gpui::TestAppContext) {
         .update(cx, |p, cx| p.open_local_buffer("/dir/file1", cx))
         .await
         .unwrap();
-    buffer
+    buffer.update(cx, |buffer, cx| {
-        .update(cx, |buffer, cx| {
+        buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx);
-            buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx);
+    });
-            buffer.save(cx)
+
-        })
+    cx.update(|cx| Project::save_buffer(buffer.clone(), cx))
         .await
         .unwrap();
 
@@ -2703,11 +2702,10 @@ async fn test_buffer_line_endings(cx: &mut gpui::TestAppContext) {
     });
 
     // Save a file with windows line endings. The file is written correctly.
-    buffer2
+    buffer2.update(cx, |buffer, cx| {
-        .update(cx, |buffer, cx| {
+        buffer.set_text("one\ntwo\nthree\nfour\n", cx);
-            buffer.set_text("one\ntwo\nthree\nfour\n", cx);
+    });
-            buffer.save(cx)
+    cx.update(|cx| Project::save_buffer(buffer2, cx))
-        })
         .await
         .unwrap();
     assert_eq!(

crates/project/src/worktree.rs

@@ -724,18 +724,55 @@ impl LocalWorktree {
         })
     }
 
+    pub fn save_buffer(
+        &self,
+        buffer_handle: ModelHandle<Buffer>,
+        path: Arc<Path>,
+        cx: &mut ModelContext<Worktree>,
+    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
+        let buffer = buffer_handle.read(cx);
+
+        let rpc = self.client.clone();
+        let buffer_id = buffer.remote_id();
+        let project_id = self.share.as_ref().map(|share| share.project_id);
+
+        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 mtime = save.await?.mtime;
+            if let Some(project_id) = project_id {
+                rpc.send(proto::BufferSaved {
+                    project_id,
+                    buffer_id,
+                    version: serialize_version(&version),
+                    mtime: Some(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);
-        let handle = cx.handle();
+
         cx.as_mut().spawn(|mut cx| async move {
             let entry = save.await?;
             let file = File {
@@ -1085,6 +1122,39 @@ impl RemoteWorktree {
         self.disconnected = true;
     }
 
+    pub fn save_buffer(
+        &self,
+        buffer_handle: ModelHandle<Buffer>,
+        cx: &mut ModelContext<Worktree>,
+    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
+        let buffer = buffer_handle.read(cx);
+        let buffer_id = buffer.remote_id();
+        let version = buffer.version();
+        let rpc = self.client.clone();
+        let project_id = self.project_id;
+        cx.as_mut().spawn(|mut cx| async move {
+            let response = rpc
+                .request(proto::SaveBuffer {
+                    project_id,
+                    buffer_id,
+                    version: serialize_version(&version),
+                })
+                .await?;
+            let version = deserialize_version(response.version);
+            let fingerprint = deserialize_fingerprint(&response.fingerprint)?;
+            let mtime = response
+                .mtime
+                .ok_or_else(|| anyhow!("missing mtime"))?
+                .into();
+
+            buffer_handle.update(&mut cx, |buffer, cx| {
+                buffer.did_save(version.clone(), fingerprint, mtime, None, cx);
+            });
+
+            Ok((version, fingerprint, mtime))
+        })
+    }
+
     pub fn update_from_remote(&mut self, update: proto::UpdateWorktree) {
         if let Some(updates_tx) = &self.updates_tx {
             updates_tx
@@ -1859,57 +1929,6 @@ impl language::File for File {
         self.is_deleted
     }
 
-    fn save(
-        &self,
-        buffer_id: u64,
-        text: Rope,
-        version: clock::Global,
-        line_ending: LineEnding,
-        cx: &mut MutableAppContext,
-    ) -> Task<Result<(clock::Global, RopeFingerprint, SystemTime)>> {
-        self.worktree.update(cx, |worktree, cx| match worktree {
-            Worktree::Local(worktree) => {
-                let rpc = worktree.client.clone();
-                let project_id = worktree.share.as_ref().map(|share| share.project_id);
-                let fingerprint = text.fingerprint();
-                let save = worktree.write_file(self.path.clone(), text, line_ending, cx);
-                cx.background().spawn(async move {
-                    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(entry.mtime.into()),
-                            fingerprint: serialize_fingerprint(fingerprint),
-                        })?;
-                    }
-                    Ok((version, fingerprint, entry.mtime))
-                })
-            }
-            Worktree::Remote(worktree) => {
-                let rpc = worktree.client.clone();
-                let project_id = worktree.project_id;
-                cx.foreground().spawn(async move {
-                    let response = rpc
-                        .request(proto::SaveBuffer {
-                            project_id,
-                            buffer_id,
-                            version: serialize_version(&version),
-                        })
-                        .await?;
-                    let version = deserialize_version(response.version);
-                    let fingerprint = deserialize_fingerprint(&response.fingerprint)?;
-                    let mtime = response
-                        .mtime
-                        .ok_or_else(|| anyhow!("missing mtime"))?
-                        .into();
-                    Ok((version, fingerprint, mtime))
-                })
-            }
-        })
-    }
-
     fn as_any(&self) -> &dyn Any {
         self
     }