From 31dac39e3453cb8e20e68336d3e6eef57710f846 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 11:12:57 -0800 Subject: [PATCH 1/8] Fix assignment of language to formerly-untitled buffers When lazy-loading a language, check if it matches plain text buffers. Co-authored-by: Nathan Sobo --- crates/project/src/project.rs | 16 ++++++++++------ crates/project/src/project_tests.rs | 29 +++++++++++++++++++++++++---- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index ec6a99edec..b1867bb623 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -554,11 +554,13 @@ impl Project { }); } - let languages = Arc::new(LanguageRegistry::test()); + let mut languages = LanguageRegistry::test(); + languages.set_executor(cx.background()); let http_client = client::test::FakeHttpClient::with_404_response(); let client = cx.update(|cx| client::Client::new(http_client.clone(), cx)); let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); - let project = cx.update(|cx| Project::local(client, user_store, languages, fs, cx)); + let project = + cx.update(|cx| Project::local(client, user_store, Arc::new(languages), fs, cx)); for path in root_paths { let (tree, _) = project .update(cx, |project, cx| { @@ -1789,20 +1791,22 @@ impl Project { while let Some(()) = subscription.next().await { if let Some(project) = project.upgrade(&cx) { project.update(&mut cx, |project, cx| { - let mut buffers_without_language = Vec::new(); + let mut plain_text_buffers = Vec::new(); let mut buffers_with_unknown_injections = Vec::new(); for buffer in project.opened_buffers.values() { if let Some(handle) = buffer.upgrade(cx) { let buffer = &handle.read(cx); - if buffer.language().is_none() { - buffers_without_language.push(handle); + if buffer.language().is_none() + || buffer.language() == Some(&*language::PLAIN_TEXT) + { + plain_text_buffers.push(handle); } else if buffer.contains_unknown_injections() { buffers_with_unknown_injections.push(handle); } } } - for buffer in buffers_without_language { + for buffer in plain_text_buffers { project.assign_language_to_buffer(&buffer, cx); project.register_buffer_with_language_server(&buffer, cx); } diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index c9e159f391..0ffa2553cd 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2130,6 +2130,20 @@ async fn test_save_as(cx: &mut gpui::TestAppContext) { fs.insert_tree("/dir", json!({})).await; let project = Project::test(fs.clone(), ["/dir".as_ref()], cx).await; + + let languages = project.read_with(cx, |project, _| project.languages().clone()); + languages.register( + "/some/path", + LanguageConfig { + name: "Rust".into(), + path_suffixes: vec!["rs".into()], + ..Default::default() + }, + tree_sitter_rust::language(), + None, + |_| Default::default(), + ); + let buffer = project.update(cx, |project, cx| { project.create_buffer("", None, cx).unwrap() }); @@ -2137,23 +2151,30 @@ async fn test_save_as(cx: &mut gpui::TestAppContext) { buffer.edit([(0..0, "abc")], None, cx); assert!(buffer.is_dirty()); assert!(!buffer.has_conflict()); + assert_eq!(buffer.language().unwrap().name().as_ref(), "Plain Text"); }); project .update(cx, |project, cx| { - project.save_buffer_as(buffer.clone(), "/dir/file1".into(), cx) + project.save_buffer_as(buffer.clone(), "/dir/file1.rs".into(), cx) }) .await .unwrap(); - assert_eq!(fs.load(Path::new("/dir/file1")).await.unwrap(), "abc"); + assert_eq!(fs.load(Path::new("/dir/file1.rs")).await.unwrap(), "abc"); + + cx.foreground().run_until_parked(); buffer.read_with(cx, |buffer, cx| { - assert_eq!(buffer.file().unwrap().full_path(cx), Path::new("dir/file1")); + assert_eq!( + buffer.file().unwrap().full_path(cx), + Path::new("dir/file1.rs") + ); assert!(!buffer.is_dirty()); assert!(!buffer.has_conflict()); + assert_eq!(buffer.language().unwrap().name().as_ref(), "Rust"); }); let opened_buffer = project .update(cx, |project, cx| { - project.open_local_buffer("/dir/file1", cx) + project.open_local_buffer("/dir/file1.rs", cx) }) .await .unwrap(); From eebce28b32fa3e3bafae779bed3895ba9fc9f865 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 12:38:04 -0800 Subject: [PATCH 2/8] Respect UpdateBufferFile messages on guest buffers without file Co-authored-by: Nathan Sobo --- crates/collab/src/tests/integration_tests.rs | 27 ++++++++++++ crates/language/src/buffer.rs | 45 ++++++++++---------- 2 files changed, 49 insertions(+), 23 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index ff9872f31f..fd67dad2e5 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/crates/collab/src/tests/integration_tests.rs @@ -2313,6 +2313,33 @@ async fn test_propagate_saves_and_fs_changes( assert_eq!(buffer.file().unwrap().path().to_str(), Some("file1.js")); assert_eq!(&*buffer.language().unwrap().name(), "JavaScript"); }); + + let new_buffer_a = project_a + .update(cx_a, |p, cx| p.create_buffer("", None, cx)) + .unwrap(); + let new_buffer_id = new_buffer_a.read_with(cx_a, |buffer, _| buffer.remote_id()); + let new_buffer_b = project_b + .update(cx_b, |p, cx| p.open_buffer_by_id(new_buffer_id, cx)) + .await + .unwrap(); + new_buffer_b.read_with(cx_b, |buffer, _| { + assert!(buffer.file().is_none()); + }); + + project_a + .update(cx_a, |project, cx| { + project.save_buffer_as(new_buffer_a, "/a/file3.rs".into(), cx) + }) + .await + .unwrap(); + + deterministic.run_until_parked(); + new_buffer_b.read_with(cx_b, |buffer, _| { + assert_eq!( + buffer.file().unwrap().path().as_ref(), + Path::new("file3.rs") + ); + }); } #[gpui::test(iterations = 10)] diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index f073fc4760..c59ec89155 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -661,36 +661,35 @@ impl Buffer { new_file: Arc, cx: &mut ModelContext, ) -> Task<()> { - let old_file = if let Some(file) = self.file.as_ref() { - file - } else { - return Task::ready(()); - }; let mut file_changed = false; let mut task = Task::ready(()); - if new_file.path() != old_file.path() { - file_changed = true; - } - - if new_file.is_deleted() { - if !old_file.is_deleted() { + if let Some(old_file) = self.file.as_ref() { + if new_file.path() != old_file.path() { file_changed = true; - if !self.is_dirty() { - cx.emit(Event::DirtyChanged); + } + + if new_file.is_deleted() { + if !old_file.is_deleted() { + file_changed = true; + if !self.is_dirty() { + cx.emit(Event::DirtyChanged); + } + } + } else { + let new_mtime = new_file.mtime(); + if new_mtime != old_file.mtime() { + file_changed = true; + + if !self.is_dirty() { + let reload = self.reload(cx).log_err().map(drop); + task = cx.foreground().spawn(reload); + } } } } else { - let new_mtime = new_file.mtime(); - if new_mtime != old_file.mtime() { - file_changed = true; - - if !self.is_dirty() { - let reload = self.reload(cx).log_err().map(drop); - task = cx.foreground().spawn(reload); - } - } - } + file_changed = true; + }; if file_changed { self.file_update_count += 1; From 76975c29a907b95dce04454554def0daee24c87b Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 15:29:54 -0800 Subject: [PATCH 3/8] Refactor: split Project::format logic into local and remote cases --- crates/project/src/project.rs | 212 +++++++++++++++++----------------- 1 file changed, 106 insertions(+), 106 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index b1867bb623..faf43ddcec 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2822,126 +2822,126 @@ impl Project { trigger: FormatTrigger, cx: &mut ModelContext, ) -> Task> { - let mut local_buffers = Vec::new(); - let mut remote_buffers = None; - for buffer_handle in buffers { - let buffer = buffer_handle.read(cx); - if let Some(file) = File::from_dyn(buffer.file()) { - if let Some(buffer_abs_path) = file.as_local().map(|f| f.abs_path(cx)) { - if let Some((_, server)) = self.language_server_for_buffer(buffer, cx) { - local_buffers.push((buffer_handle, buffer_abs_path, server.clone())); - } - } else { - remote_buffers.get_or_insert(Vec::new()).push(buffer_handle); - } - } else { - return Task::ready(Ok(Default::default())); - } - } + if self.is_local() { + let mut buffers_with_paths_and_servers = buffers + .into_iter() + .filter_map(|buffer_handle| { + let buffer = buffer_handle.read(cx); + let file = File::from_dyn(buffer.file())?; + let buffer_abs_path = file.as_local()?.abs_path(cx); + let (_, server) = self.language_server_for_buffer(buffer, cx)?; + Some((buffer_handle, buffer_abs_path, server.clone())) + }) + .collect::>(); - let remote_buffers = self.remote_id().zip(remote_buffers); - let client = self.client.clone(); - - cx.spawn(|this, mut cx| async move { - let mut project_transaction = ProjectTransaction::default(); - - if let Some((project_id, remote_buffers)) = remote_buffers { - let response = client - .request(proto::FormatBuffers { - project_id, - trigger: trigger as i32, - buffer_ids: remote_buffers - .iter() - .map(|buffer| buffer.read_with(&cx, |buffer, _| buffer.remote_id())) - .collect(), - }) - .await? - .transaction - .ok_or_else(|| anyhow!("missing transaction"))?; - project_transaction = this - .update(&mut cx, |this, cx| { - this.deserialize_project_transaction(response, push_to_history, cx) - }) - .await?; - } - - // Do not allow multiple concurrent formatting requests for the - // same buffer. - this.update(&mut cx, |this, _| { - local_buffers - .retain(|(buffer, _, _)| this.buffers_being_formatted.insert(buffer.id())); - }); - let _cleanup = defer({ - let this = this.clone(); - let mut cx = cx.clone(); - let local_buffers = &local_buffers; - move || { - this.update(&mut cx, |this, _| { - for (buffer, _, _) in local_buffers { - this.buffers_being_formatted.remove(&buffer.id()); - } - }); - } - }); - - for (buffer, buffer_abs_path, language_server) in &local_buffers { - let (format_on_save, formatter, tab_size) = buffer.read_with(&cx, |buffer, cx| { - let settings = cx.global::(); - let language_name = buffer.language().map(|language| language.name()); - ( - settings.format_on_save(language_name.as_deref()), - settings.formatter(language_name.as_deref()), - settings.tab_size(language_name.as_deref()), - ) + cx.spawn(|this, mut cx| async move { + // Do not allow multiple concurrent formatting requests for the + // same buffer. + this.update(&mut cx, |this, _| { + buffers_with_paths_and_servers + .retain(|(buffer, _, _)| this.buffers_being_formatted.insert(buffer.id())); }); - let transaction = match (formatter, format_on_save) { - (_, FormatOnSave::Off) if trigger == FormatTrigger::Save => continue, + let _cleanup = defer({ + let this = this.clone(); + let mut cx = cx.clone(); + let local_buffers = &buffers_with_paths_and_servers; + move || { + this.update(&mut cx, |this, _| { + for (buffer, _, _) in local_buffers { + this.buffers_being_formatted.remove(&buffer.id()); + } + }); + } + }); - (Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off) - | (_, FormatOnSave::LanguageServer) => Self::format_via_lsp( - &this, - &buffer, - &buffer_abs_path, - &language_server, - tab_size, - &mut cx, - ) - .await - .context("failed to format via language server")?, + let mut project_transaction = ProjectTransaction::default(); + for (buffer, buffer_abs_path, language_server) in &buffers_with_paths_and_servers { + let (format_on_save, formatter, tab_size) = + buffer.read_with(&cx, |buffer, cx| { + let settings = cx.global::(); + let language_name = buffer.language().map(|language| language.name()); + ( + settings.format_on_save(language_name.as_deref()), + settings.formatter(language_name.as_deref()), + settings.tab_size(language_name.as_deref()), + ) + }); - ( - Formatter::External { command, arguments }, - FormatOnSave::On | FormatOnSave::Off, - ) - | (_, FormatOnSave::External { command, arguments }) => { - Self::format_via_external_command( + let transaction = match (formatter, format_on_save) { + (_, FormatOnSave::Off) if trigger == FormatTrigger::Save => continue, + + (Formatter::LanguageServer, FormatOnSave::On | FormatOnSave::Off) + | (_, FormatOnSave::LanguageServer) => Self::format_via_lsp( + &this, &buffer, &buffer_abs_path, - &command, - &arguments, + &language_server, + tab_size, &mut cx, ) .await - .context(format!( - "failed to format via external command {:?}", - command - ))? - } - }; + .context("failed to format via language server")?, - if let Some(transaction) = transaction { - if !push_to_history { - buffer.update(&mut cx, |buffer, _| { - buffer.forget_transaction(transaction.id) - }); + ( + Formatter::External { command, arguments }, + FormatOnSave::On | FormatOnSave::Off, + ) + | (_, FormatOnSave::External { command, arguments }) => { + Self::format_via_external_command( + &buffer, + &buffer_abs_path, + &command, + &arguments, + &mut cx, + ) + .await + .context(format!( + "failed to format via external command {:?}", + command + ))? + } + }; + + if let Some(transaction) = transaction { + if !push_to_history { + buffer.update(&mut cx, |buffer, _| { + buffer.forget_transaction(transaction.id) + }); + } + project_transaction.0.insert(buffer.clone(), transaction); } - project_transaction.0.insert(buffer.clone(), transaction); } - } - Ok(project_transaction) - }) + Ok(project_transaction) + }) + } else { + let remote_id = self.remote_id(); + let client = self.client.clone(); + cx.spawn(|this, mut cx| async move { + let mut project_transaction = ProjectTransaction::default(); + if let Some(project_id) = remote_id { + let response = client + .request(proto::FormatBuffers { + project_id, + trigger: trigger as i32, + buffer_ids: buffers + .iter() + .map(|buffer| buffer.read_with(&cx, |buffer, _| buffer.remote_id())) + .collect(), + }) + .await? + .transaction + .ok_or_else(|| anyhow!("missing transaction"))?; + project_transaction = this + .update(&mut cx, |this, cx| { + this.deserialize_project_transaction(response, push_to_history, cx) + }) + .await?; + } + Ok(project_transaction) + }) + } } async fn format_via_lsp( From de6eb00e2bca6acf7a0fb25e97b219b20d65f1a1 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 15:41:39 -0800 Subject: [PATCH 4/8] Start work on making save and save_as code paths more similar --- crates/editor/src/items.rs | 35 +++++++++---------------------- crates/editor/src/multi_buffer.rs | 15 ------------- crates/project/src/project.rs | 33 ++++++++++++++++++++++++++++- 3 files changed, 42 insertions(+), 41 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index c52d00b7e3..2c8503b8c7 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -2,12 +2,10 @@ use crate::{ display_map::ToDisplayPoint, link_go_to_definition::hide_link_definition, movement::surrounding_word, persistence::DB, scroll::ScrollAnchor, Anchor, Autoscroll, Editor, Event, ExcerptId, ExcerptRange, MultiBuffer, MultiBufferSnapshot, NavigationData, ToPoint as _, - FORMAT_TIMEOUT, }; use anyhow::{anyhow, Context, Result}; use collections::HashSet; use futures::future::try_join_all; -use futures::FutureExt; use gpui::{ elements::*, geometry::vector::vec2f, AppContext, Entity, ModelHandle, MutableAppContext, RenderContext, Subscription, Task, View, ViewContext, ViewHandle, WeakViewHandle, @@ -16,7 +14,7 @@ use language::{ proto::serialize_anchor as serialize_text_anchor, Bias, Buffer, OffsetRangeExt, Point, SelectionGoal, }; -use project::{FormatTrigger, Item as _, Project, ProjectPath}; +use project::{Item as _, Project, ProjectPath}; use rpc::proto::{self, update_view}; use settings::Settings; use smallvec::SmallVec; @@ -613,30 +611,17 @@ impl Item for Editor { let buffer = self.buffer().clone(); let buffers = buffer.read(cx).all_buffers(); - let mut timeout = cx.background().timer(FORMAT_TIMEOUT).fuse(); - let format = project.update(cx, |project, cx| { - project.format(buffers, true, FormatTrigger::Save, cx) - }); + let save = project.update(cx, |project, cx| project.save_buffers(buffers, cx)); cx.spawn(|_, mut cx| async move { - let transaction = futures::select_biased! { - _ = timeout => { - log::warn!("timed out waiting for formatting"); - None - } - transaction = format.log_err().fuse() => transaction, - }; - - buffer - .update(&mut cx, |buffer, cx| { - if let Some(transaction) = transaction { - if !buffer.is_singleton() { - buffer.push_transaction(&transaction.0); - } + let (format_transaction, save) = save.await; + buffer.update(&mut cx, |buffer, _| { + if let Some(transaction) = format_transaction { + if !buffer.is_singleton() { + buffer.push_transaction(&transaction.0); } - - buffer.save(cx) - }) - .await?; + } + }); + save.await?; Ok(()) }) } diff --git a/crates/editor/src/multi_buffer.rs b/crates/editor/src/multi_buffer.rs index 908e5c827d..9e0e22ad40 100644 --- a/crates/editor/src/multi_buffer.rs +++ b/crates/editor/src/multi_buffer.rs @@ -1,7 +1,6 @@ mod anchor; pub use anchor::{Anchor, AnchorRangeExt}; -use anyhow::Result; use clock::ReplicaId; use collections::{BTreeMap, Bound, HashMap, HashSet}; use futures::{channel::mpsc, SinkExt}; @@ -1279,20 +1278,6 @@ impl MultiBuffer { .map(|state| state.buffer.clone()) } - pub fn save(&mut self, cx: &mut ModelContext) -> Task> { - let mut save_tasks = Vec::new(); - for BufferState { buffer, .. } in self.buffers.borrow().values() { - save_tasks.push(buffer.update(cx, |buffer, cx| buffer.save(cx))); - } - - cx.spawn(|_, _| async move { - for save in save_tasks { - save.await?; - } - Ok(()) - }) - } - pub fn is_completion_trigger(&self, position: T, text: &str, cx: &AppContext) -> bool where T: ToOffset, diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index faf43ddcec..7eb2b61990 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -12,7 +12,7 @@ use clock::ReplicaId; use collections::{hash_map, BTreeMap, HashMap, HashSet}; use futures::{ channel::{mpsc, oneshot}, - future::Shared, + future::{try_join_all, Shared}, AsyncWriteExt, Future, FutureExt, StreamExt, TryFutureExt, }; use gpui::{ @@ -1428,6 +1428,37 @@ impl Project { } } + pub fn save_buffers( + &mut self, + buffers: HashSet>, + cx: &mut ModelContext, + ) -> Task<(Option, Task>)> { + const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); + + let mut timeout = cx.background().timer(FORMAT_TIMEOUT).fuse(); + let format = self.format(buffers.clone(), true, FormatTrigger::Save, cx); + cx.spawn(|_, cx| async move { + let transaction = futures::select_biased! { + _ = timeout => { + log::warn!("timed out waiting for formatting"); + None + } + transaction = format.log_err().fuse() => transaction, + }; + + ( + 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(()) + }), + ) + }) + } + pub fn save_buffer_as( &mut self, buffer: ModelHandle, From 3a7cfc39012e395b2b9f098e84f7be4d17a551d7 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 16:53:18 -0800 Subject: [PATCH 5/8] Move the save and save_as code paths close together --- crates/collab/src/tests/integration_tests.rs | 4 +- .../src/tests/randomized_integration_tests.rs | 5 +- crates/editor/src/items.rs | 29 +---- crates/language/src/buffer.rs | 36 ----- crates/project/src/project.rs | 63 +++++---- crates/project/src/project_tests.rs | 34 +++-- crates/project/src/worktree.rs | 123 ++++++++++-------- 7 files changed, 125 insertions(+), 169 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index fd67dad2e5..394dcd808e 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/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, _| { diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 4783957dbb..434006c5c1 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/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 diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 2c8503b8c7..aac8701a22 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -608,20 +608,11 @@ impl Item for Editor { cx: &mut ViewContext, ) -> Task> { self.report_event("save editor", cx); - - let buffer = self.buffer().clone(); - let buffers = buffer.read(cx).all_buffers(); - let save = project.update(cx, |project, cx| project.save_buffers(buffers, cx)); + let format = self.perform_format(project.clone(), cx); + let buffers = self.buffer().clone().read(cx).all_buffers(); cx.spawn(|_, mut cx| async move { - let (format_transaction, save) = save.await; - buffer.update(&mut cx, |buffer, _| { - if let Some(transaction) = format_transaction { - if !buffer.is_singleton() { - buffer.push_transaction(&transaction.0); - } - } - }); - save.await?; + format.await?; + cx.update(|cx| Project::save_buffers(buffers, cx)).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> { - todo!() - } - fn as_any(&self) -> &dyn std::any::Any { todo!() } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index c59ec89155..ad9345a6bd 100644 --- a/crates/language/src/buffer.rs +++ b/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>; - 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, - ) -> Task> { - 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 } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 7eb2b61990..3cde06e317 100644 --- a/crates/project/src/project.rs +++ b/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, - Unclipped, + Operation, Patch, PointUtf16, RopeFingerprint, TextBufferSnapshot, ToOffset, ToPointUtf16, + 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>, - cx: &mut ModelContext, - ) -> Task<(Option, Task>)> { - const FORMAT_TIMEOUT: Duration = Duration::from_secs(2); + cx: &mut MutableAppContext, + ) -> Task> { + 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(); - let format = self.format(buffers.clone(), true, FormatTrigger::Save, cx); - cx.spawn(|_, cx| async move { - let transaction = futures::select_biased! { - _ = timeout => { - log::warn!("timed out waiting for formatting"); - None - } - transaction = format.log_err().fuse() => transaction, - }; - - ( - 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(()) - }), - ) + pub fn save_buffer( + buffer: ModelHandle, + cx: &mut MutableAppContext, + ) -> Task> { + let Some(file) = File::from_dyn(buffer.read(cx).file()) else { + return Task::ready(Err(anyhow!("buffer doesn't have a file"))); + }; + 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::Remote(worktree) => worktree.save_buffer(buffer, cx), }) } @@ -1476,11 +1473,9 @@ impl Project { } let (worktree, path) = worktree_task.await?; worktree - .update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .save_buffer_as(buffer.clone(), path, cx) + .update(&mut cx, |worktree, cx| match worktree { + Worktree::Local(worktree) => worktree.save_buffer_as(buffer.clone(), path, cx), + Worktree::Remote(_) => panic!("cannot remote buffers as new files"), }) .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, diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 0ffa2553cd..2a88714b18 100644 --- a/crates/project/src/project_tests.rs +++ b/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 - .update(cx, |buffer, cx| buffer.save(cx)) + cx.update(|cx| Project::save_buffer(toml_buffer, 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 - .update(cx, |buffer, cx| { - assert_eq!(buffer.text(), "the old contents"); - buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx); - buffer.save(cx) - }) + buffer.update(cx, |buffer, cx| { + assert_eq!(buffer.text(), "the old contents"); + buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, 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 - .update(cx, |buffer, cx| { - buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx); - buffer.save(cx) - }) + buffer.update(cx, |buffer, cx| { + buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, 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 - .update(cx, |buffer, cx| { - buffer.set_text("one\ntwo\nthree\nfour\n", cx); - buffer.save(cx) - }) + buffer2.update(cx, |buffer, cx| { + buffer.set_text("one\ntwo\nthree\nfour\n", cx); + }); + cx.update(|cx| Project::save_buffer(buffer2, cx)) .await .unwrap(); assert_eq!( diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index d743ba2031..5edb224618 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -724,18 +724,55 @@ impl LocalWorktree { }) } + pub fn save_buffer( + &self, + buffer_handle: ModelHandle, + path: Arc, + cx: &mut ModelContext, + ) -> Task> { + 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, 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); - 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, + cx: &mut ModelContext, + ) -> Task> { + 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> { - 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 } From cdf64b6cad090cdac1b1886328883095addb4995 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 17 Feb 2023 17:08:28 -0800 Subject: [PATCH 6/8] 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 394dcd808e..8b92216734 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 3cde06e317..ba5ca82c02 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 5edb224618..9f4446f060 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)) }) } From 56b7eb6b6f2b83529906fa876bb56d25e86452b5 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 20 Feb 2023 09:33:02 -0800 Subject: [PATCH 7/8] Only send UpdateBufferFile messages for buffers whose files have changed Send that message when saving a buffer as a new path. --- crates/language/src/buffer.rs | 5 --- crates/project/src/project.rs | 23 +++++++------ crates/project/src/project_tests.rs | 1 - crates/project/src/worktree.rs | 50 +++++++++++++++++------------ 4 files changed, 43 insertions(+), 36 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index ad9345a6bd..e34b3d8282 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -549,16 +549,11 @@ impl Buffer { version: clock::Global, fingerprint: RopeFingerprint, mtime: SystemTime, - new_file: Option>, cx: &mut ModelContext, ) { self.saved_version = version; self.saved_version_fingerprint = fingerprint; self.saved_mtime = mtime; - if let Some(new_file) = new_file { - self.file = Some(new_file); - self.file_update_count += 1; - } cx.emit(Event::Saved); cx.notify(); } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index ba5ca82c02..3a0e0a43c0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -4461,16 +4461,19 @@ impl Project { renamed_buffers.push((cx.handle(), old_path)); } - if let Some(project_id) = self.remote_id() { - self.client - .send(proto::UpdateBufferFile { - project_id, - buffer_id: *buffer_id as u64, - file: Some(new_file.to_proto()), - }) - .log_err(); + if new_file != *old_file { + if let Some(project_id) = self.remote_id() { + self.client + .send(proto::UpdateBufferFile { + project_id, + buffer_id: *buffer_id as u64, + file: Some(new_file.to_proto()), + }) + .log_err(); + } + + buffer.file_updated(Arc::new(new_file), cx).detach(); } - buffer.file_updated(Arc::new(new_file), cx).detach(); } }); } else { @@ -6054,7 +6057,7 @@ impl Project { .and_then(|buffer| buffer.upgrade(cx)); if let Some(buffer) = buffer { buffer.update(cx, |buffer, cx| { - buffer.did_save(version, fingerprint, mtime, None, cx); + buffer.did_save(version, fingerprint, mtime, cx); }); } Ok(()) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 2a88714b18..fa3bf9dff8 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2482,7 +2482,6 @@ async fn test_buffer_is_dirty(cx: &mut gpui::TestAppContext) { buffer.version(), buffer.as_rope().fingerprint(), buffer.file().unwrap().mtime(), - None, cx, ); }); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 9f4446f060..8b622ab607 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -20,6 +20,7 @@ use gpui::{ executor, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext, Task, }; +use language::File as _; use language::{ proto::{ deserialize_fingerprint, deserialize_version, serialize_fingerprint, serialize_line_ending, @@ -728,7 +729,7 @@ impl LocalWorktree { &self, buffer_handle: ModelHandle, path: Arc, - replace_file: bool, + has_changed_file: bool, cx: &mut ModelContext, ) -> Task> { let handle = cx.handle(); @@ -746,6 +747,32 @@ impl LocalWorktree { cx.as_mut().spawn(|mut cx| async move { let entry = save.await?; + if has_changed_file { + let new_file = Arc::new(File { + entry_id: entry.id, + worktree: handle, + path: entry.path, + mtime: entry.mtime, + is_local: true, + is_deleted: false, + }); + + if let Some(project_id) = project_id { + rpc.send(proto::UpdateBufferFile { + project_id, + buffer_id, + file: Some(new_file.to_proto()), + }) + .log_err(); + } + + buffer_handle.update(&mut cx, |buffer, cx| { + if has_changed_file { + buffer.file_updated(new_file, cx).detach(); + } + }); + } + if let Some(project_id) = project_id { rpc.send(proto::BufferSaved { project_id, @@ -757,24 +784,7 @@ impl LocalWorktree { } buffer_handle.update(&mut cx, |buffer, 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, - ); + buffer.did_save(version.clone(), fingerprint, entry.mtime, cx); }); Ok((version, fingerprint, entry.mtime)) @@ -1137,7 +1147,7 @@ impl RemoteWorktree { .into(); buffer_handle.update(&mut cx, |buffer, cx| { - buffer.did_save(version.clone(), fingerprint, mtime, None, cx); + buffer.did_save(version.clone(), fingerprint, mtime, cx); }); Ok((version, fingerprint, mtime)) From 010eba509c07b642fea09a49e9932b90abd3be89 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Mon, 20 Feb 2023 09:42:44 -0800 Subject: [PATCH 8/8] Make Project::save_buffer and ::save_buffers into methods --- crates/collab/src/tests/integration_tests.rs | 4 ++-- .../src/tests/randomized_integration_tests.rs | 2 +- crates/editor/src/items.rs | 6 ++++-- crates/project/src/project.rs | 17 ++++++++++------- crates/project/src/project_tests.rs | 12 ++++++++---- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/crates/collab/src/tests/integration_tests.rs b/crates/collab/src/tests/integration_tests.rs index 8b92216734..f2cb2eddbb 100644 --- a/crates/collab/src/tests/integration_tests.rs +++ b/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 = cx_b.update(|cx| Project::save_buffer(buffer_b.clone(), cx)); + let save_b = project_b.update(cx_b, |project, 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!( @@ -2917,7 +2917,7 @@ async fn test_buffer_conflict_after_save( assert!(!buf.has_conflict()); }); - cx_b.update(|cx| Project::save_buffer(buffer_b.clone(), cx)) + project_b.update(cx_b, |project, cx| project.save_buffer(buffer_b.clone(), cx)) .await .unwrap(); cx_a.foreground().forbid_parking(); diff --git a/crates/collab/src/tests/randomized_integration_tests.rs b/crates/collab/src/tests/randomized_integration_tests.rs index 434006c5c1..950f12d186 100644 --- a/crates/collab/src/tests/randomized_integration_tests.rs +++ b/crates/collab/src/tests/randomized_integration_tests.rs @@ -1073,7 +1073,7 @@ async fn randomly_query_and_mutate_buffers( ); buffer.version() }); - let save = cx.update(|cx| Project::save_buffer(buffer, cx)); + let save = project.update(cx, |project, cx| project.save_buffer(buffer, cx)); let save = cx.background().spawn(async move { let (saved_version, _, _) = save .await diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index aac8701a22..c3a446faa7 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -610,9 +610,11 @@ impl Item for Editor { self.report_event("save editor", cx); let format = self.perform_format(project.clone(), cx); let buffers = self.buffer().clone().read(cx).all_buffers(); - cx.spawn(|_, mut cx| async move { + cx.as_mut().spawn(|mut cx| async move { format.await?; - cx.update(|cx| Project::save_buffers(buffers, cx)).await?; + project + .update(&mut cx, |project, cx| project.save_buffers(buffers, cx)) + .await?; Ok(()) }) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 3a0e0a43c0..8ed37a003c 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1429,21 +1429,23 @@ impl Project { } pub fn save_buffers( + &self, buffers: HashSet>, - cx: &mut MutableAppContext, + cx: &mut ModelContext, ) -> Task> { - cx.spawn(|mut cx| async move { + cx.spawn(|this, mut cx| async move { let save_tasks = buffers .into_iter() - .map(|buffer| cx.update(|cx| Self::save_buffer(buffer, cx))); + .map(|buffer| this.update(&mut cx, |this, cx| this.save_buffer(buffer, cx))); try_join_all(save_tasks).await?; Ok(()) }) } pub fn save_buffer( + &self, buffer: ModelHandle, - cx: &mut MutableAppContext, + cx: &mut ModelContext, ) -> Task> { let Some(file) = File::from_dyn(buffer.read(cx).file()) else { return Task::ready(Err(anyhow!("buffer doesn't have a file"))); @@ -1460,7 +1462,7 @@ impl Project { &mut self, buffer: ModelHandle, abs_path: PathBuf, - cx: &mut ModelContext, + cx: &mut ModelContext, ) -> Task> { let worktree_task = self.find_or_create_local_worktree(&abs_path, true, cx); let old_path = @@ -5186,8 +5188,9 @@ impl Project { }) .await; - let (saved_version, fingerprint, mtime) = - cx.update(|cx| Self::save_buffer(buffer, cx)).await?; + let (saved_version, fingerprint, mtime) = this + .update(&mut cx, |this, cx| this.save_buffer(buffer, cx)) + .await?; Ok(proto::BufferSaved { project_id, buffer_id, diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index fa3bf9dff8..2f9f92af4e 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -243,7 +243,8 @@ async fn test_managing_language_servers( ); // Save notifications are reported to all servers. - cx.update(|cx| Project::save_buffer(toml_buffer, cx)) + project + .update(cx, |project, cx| project.save_buffer(toml_buffer, cx)) .await .unwrap(); assert_eq!( @@ -2087,7 +2088,8 @@ async fn test_save_file(cx: &mut gpui::TestAppContext) { buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx); }); - cx.update(|cx| Project::save_buffer(buffer.clone(), cx)) + project + .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)) .await .unwrap(); @@ -2115,7 +2117,8 @@ async fn test_save_in_single_file_worktree(cx: &mut gpui::TestAppContext) { buffer.edit([(0..0, "a line of text.\n".repeat(10 * 1024))], None, cx); }); - cx.update(|cx| Project::save_buffer(buffer.clone(), cx)) + project + .update(cx, |project, cx| project.save_buffer(buffer.clone(), cx)) .await .unwrap(); @@ -2704,7 +2707,8 @@ async fn test_buffer_line_endings(cx: &mut gpui::TestAppContext) { buffer2.update(cx, |buffer, cx| { buffer.set_text("one\ntwo\nthree\nfour\n", cx); }); - cx.update(|cx| Project::save_buffer(buffer2, cx)) + project + .update(cx, |project, cx| project.save_buffer(buffer2, cx)) .await .unwrap(); assert_eq!(