From 6751bd9d783e5f67e4c7d45ca024a9603330acf3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 12:23:17 -0800 Subject: [PATCH 01/12] Change integration tests to open buffers via the project --- crates/project/src/project.rs | 19 +++- crates/project/src/worktree.rs | 16 +--- crates/server/src/rpc.rs | 153 +++++++++++++++------------------ 3 files changed, 88 insertions(+), 100 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index f01a875011ef29e733a71815eb056816e3caa62c..09925badf62653e9fdf84b13b1433c80fbdb18b4 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -476,9 +476,10 @@ impl Project { pub fn open_buffer( &mut self, - path: ProjectPath, + path: impl Into, cx: &mut ModelContext, ) -> Task>> { + let path = path.into(); let worktree = if let Some(worktree) = self.worktree_for_id(path.worktree_id, cx) { worktree } else { @@ -520,6 +521,13 @@ impl Project { }) } + #[cfg(any(test, feature = "test-support"))] + pub fn has_open_buffer(&self, path: impl Into, cx: &AppContext) -> bool { + let path = path.into(); + self.worktree_for_id(path.worktree_id, cx) + .map_or(false, |tree| tree.read(cx).has_open_buffer(path.path, cx)) + } + fn assign_language_to_buffer( &mut self, worktree: ModelHandle, @@ -1470,6 +1478,15 @@ impl Collaborator { } } +impl> From<(WorktreeId, P)> for ProjectPath { + fn from((worktree_id, path): (WorktreeId, P)) -> Self { + Self { + worktree_id, + path: path.as_ref().into(), + } + } +} + #[cfg(test)] mod tests { use super::{Event, *}; diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index e0a54e45ed776e9cbdb0743e6254c0fa714a0d97..c07178e6d96c9a58da64ecf16c682760b873e417 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -322,14 +322,14 @@ impl Worktree { .map(|(path, summary)| (path.0.clone(), summary.clone())) } - pub fn loading_buffers<'a>(&'a mut self) -> &'a mut LoadingBuffers { + pub(crate) fn loading_buffers<'a>(&'a mut self) -> &'a mut LoadingBuffers { match self { Worktree::Local(worktree) => &mut worktree.loading_buffers, Worktree::Remote(worktree) => &mut worktree.loading_buffers, } } - pub fn open_buffer( + pub(crate) fn open_buffer( &mut self, path: impl AsRef, cx: &mut ModelContext, @@ -391,7 +391,7 @@ impl Worktree { } #[cfg(feature = "test-support")] - pub fn has_open_buffer(&self, path: impl AsRef, cx: &AppContext) -> bool { + pub(crate) fn has_open_buffer(&self, path: impl AsRef, cx: &AppContext) -> bool { let mut open_buffers: Box> = match self { Worktree::Local(worktree) => Box::new(worktree.open_buffers.values()), Worktree::Remote(worktree) => { @@ -1577,16 +1577,6 @@ impl RemoteWorktree { }) } - pub fn close_all_buffers(&mut self, cx: &mut MutableAppContext) { - for (_, buffer) in self.open_buffers.drain() { - if let RemoteBuffer::Loaded(buffer) = buffer { - if let Some(buffer) = buffer.upgrade(cx) { - buffer.update(cx, |buffer, cx| buffer.close(cx)) - } - } - } - } - fn snapshot(&self) -> Snapshot { self.snapshot.clone() } diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 3ad10421363ff31eb1f68ecc6c52783aa3f94e22..cca6a3da4b0bff24c35e2bc7dcca8ba0997b2c49 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -1166,14 +1166,13 @@ mod tests { }) .await .unwrap(); + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -1188,7 +1187,6 @@ mod tests { ) .await .unwrap(); - let worktree_b = project_b.update(&mut cx_b, |p, cx| p.worktrees(cx).next().unwrap()); let replica_id_b = project_b.read_with(&cx_b, |project, _| { assert_eq!( @@ -1214,25 +1212,26 @@ mod tests { .await; // Open the same file as client B and client A. - let buffer_b = worktree_b - .update(&mut cx_b, |worktree, cx| worktree.open_buffer("b.txt", cx)) + let buffer_b = project_b + .update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "b.txt"), cx)) .await - .unwrap() - .0; + .unwrap(); let buffer_b = cx_b.add_model(|cx| MultiBuffer::singleton(buffer_b, cx)); buffer_b.read_with(&cx_b, |buf, cx| { assert_eq!(buf.read(cx).text(), "b-contents") }); - worktree_a.read_with(&cx_a, |tree, cx| assert!(tree.has_open_buffer("b.txt", cx))); - let buffer_a = worktree_a - .update(&mut cx_a, |tree, cx| tree.open_buffer("b.txt", cx)) + project_a.read_with(&cx_a, |project, cx| { + assert!(project.has_open_buffer((worktree_id, "b.txt"), cx)) + }); + let buffer_a = project_a + .update(&mut cx_a, |p, cx| p.open_buffer((worktree_id, "b.txt"), cx)) .await - .unwrap() - .0; + .unwrap(); let editor_b = cx_b.add_view(window_b, |cx| { Editor::for_buffer(buffer_b, Arc::new(|cx| EditorSettings::test(cx)), cx) }); + // TODO // // Create a selection set as client B and see that selection set as client A. // buffer_a @@ -1256,8 +1255,10 @@ mod tests { // Close the buffer as client A, see that the buffer is closed. cx_a.update(move |_| drop(buffer_a)); - worktree_a - .condition(&cx_a, |tree, cx| !tree.has_open_buffer("b.txt", cx)) + project_a + .condition(&cx_a, |project, cx| { + !project.has_open_buffer((worktree_id, "b.txt"), cx) + }) .await; // Dropping the client B's project removes client B from client A's collaborators. @@ -1306,11 +1307,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); assert!(worktree_a.read_with(&cx_a, |tree, _| tree.as_local().unwrap().is_shared())); @@ -1326,13 +1326,12 @@ mod tests { ) .await .unwrap(); - - let worktree_b = project_b.read_with(&cx_b, |p, cx| p.worktrees(cx).next().unwrap()); - worktree_b - .update(&mut cx_b, |tree, cx| tree.open_buffer("a.txt", cx)) + project_b + .update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) .await .unwrap(); + // Unshare the project as client A project_a .update(&mut cx_a, |project, cx| project.unshare(cx)) .await @@ -1349,6 +1348,7 @@ mod tests { .await .unwrap(); assert!(worktree_a.read_with(&cx_a, |tree, _| tree.as_local().unwrap().is_shared())); + let project_c = Project::remote( project_id, client_b.clone(), @@ -1359,9 +1359,8 @@ mod tests { ) .await .unwrap(); - let worktree_c = project_c.read_with(&cx_b, |p, cx| p.worktrees(cx).next().unwrap()); - worktree_c - .update(&mut cx_b, |tree, cx| tree.open_buffer("a.txt", cx)) + project_c + .update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) .await .unwrap(); } @@ -1410,11 +1409,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -1439,29 +1437,26 @@ mod tests { ) .await .unwrap(); - - // Open and edit a buffer as both guests B and C. let worktree_b = project_b.read_with(&cx_b, |p, cx| p.worktrees(cx).next().unwrap()); let worktree_c = project_c.read_with(&cx_c, |p, cx| p.worktrees(cx).next().unwrap()); - let buffer_b = worktree_b - .update(&mut cx_b, |tree, cx| tree.open_buffer("file1", cx)) + + // Open and edit a buffer as both guests B and C. + let buffer_b = project_b + .update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "file1"), cx)) .await - .unwrap() - .0; - let buffer_c = worktree_c - .update(&mut cx_c, |tree, cx| tree.open_buffer("file1", cx)) + .unwrap(); + let buffer_c = project_c + .update(&mut cx_c, |p, cx| p.open_buffer((worktree_id, "file1"), cx)) .await - .unwrap() - .0; + .unwrap(); buffer_b.update(&mut cx_b, |buf, cx| buf.edit([0..0], "i-am-b, ", cx)); buffer_c.update(&mut cx_c, |buf, cx| buf.edit([0..0], "i-am-c, ", cx)); // Open and edit that buffer as the host. - let buffer_a = worktree_a - .update(&mut cx_a, |tree, cx| tree.open_buffer("file1", cx)) + let buffer_a = project_a + .update(&mut cx_a, |p, cx| p.open_buffer((worktree_id, "file1"), cx)) .await - .unwrap() - .0; + .unwrap(); buffer_a .condition(&mut cx_a, |buf, _| buf.text() == "i-am-c, i-am-b, ") @@ -1564,11 +1559,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -1586,11 +1580,10 @@ mod tests { let worktree_b = project_b.update(&mut cx_b, |p, cx| p.worktrees(cx).next().unwrap()); // Open a buffer as client B - let buffer_b = worktree_b - .update(&mut cx_b, |worktree, cx| worktree.open_buffer("a.txt", cx)) + let buffer_b = project_b + .update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) .await - .unwrap() - .0; + .unwrap(); let mtime = buffer_b.read_with(&cx_b, |buf, _| buf.file().unwrap().mtime()); buffer_b.update(&mut cx_b, |buf, cx| buf.edit([0..0], "world ", cx)); @@ -1661,11 +1654,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -1680,26 +1672,24 @@ mod tests { ) .await .unwrap(); - let worktree_b = project_b.update(&mut cx_b, |p, cx| p.worktrees(cx).next().unwrap()); // Open a buffer as client A - let buffer_a = worktree_a - .update(&mut cx_a, |tree, cx| tree.open_buffer("a.txt", cx)) + let buffer_a = project_a + .update(&mut cx_a, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) .await - .unwrap() - .0; + .unwrap(); // Start opening the same buffer as client B let buffer_b = cx_b .background() - .spawn(worktree_b.update(&mut cx_b, |worktree, cx| worktree.open_buffer("a.txt", cx))); + .spawn(project_b.update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))); task::yield_now().await; // Edit the buffer as client A while client B is still opening it. buffer_a.update(&mut cx_a, |buf, cx| buf.edit([0..0], "z", cx)); let text = buffer_a.read_with(&cx_a, |buf, _| buf.text()); - let buffer_b = buffer_b.await.unwrap().0; + let buffer_b = buffer_b.await.unwrap(); buffer_b.condition(&cx_b, |buf, _| buf.text() == text).await; } @@ -1744,11 +1734,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -1763,7 +1752,6 @@ mod tests { ) .await .unwrap(); - let worktree_b = project_b.update(&mut cx_b, |p, cx| p.worktrees(cx).next().unwrap()); // See that a guest has joined as client A. project_a @@ -1773,7 +1761,7 @@ mod tests { // Begin opening a buffer as client B, but leave the project before the open completes. let buffer_b = cx_b .background() - .spawn(worktree_b.update(&mut cx_b, |worktree, cx| worktree.open_buffer("a.txt", cx))); + .spawn(project_b.update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx))); cx_b.update(|_| drop(project_b)); drop(buffer_b); @@ -1911,12 +1899,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -2041,13 +2027,11 @@ mod tests { .await; // Open the file with the errors on client B. They should be present. - let worktree_b = project_b.update(&mut cx_b, |p, cx| p.worktrees(cx).next().unwrap()); let buffer_b = cx_b .background() - .spawn(worktree_b.update(&mut cx_b, |worktree, cx| worktree.open_buffer("a.rs", cx))) + .spawn(project_b.update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.rs"), cx))) .await - .unwrap() - .0; + .unwrap(); buffer_b.read_with(&cx_b, |buffer, _| { assert_eq!( @@ -2135,11 +2119,10 @@ mod tests { worktree_a .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) .await; - let project_id = project_a - .update(&mut cx_a, |project, _| project.next_remote_id()) - .await; + let project_id = project_a.update(&mut cx_a, |p, _| p.next_remote_id()).await; + let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); project_a - .update(&mut cx_a, |project, cx| project.share(cx)) + .update(&mut cx_a, |p, cx| p.share(cx)) .await .unwrap(); @@ -2156,13 +2139,11 @@ mod tests { .unwrap(); // Open the file to be formatted on client B. - let worktree_b = project_b.update(&mut cx_b, |p, cx| p.worktrees(cx).next().unwrap()); let buffer_b = cx_b .background() - .spawn(worktree_b.update(&mut cx_b, |worktree, cx| worktree.open_buffer("a.rs", cx))) + .spawn(project_b.update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.rs"), cx))) .await - .unwrap() - .0; + .unwrap(); let format = buffer_b.update(&mut cx_b, |buffer, cx| buffer.format(cx)); let (request_id, _) = fake_language_server From 7de26302eccd6b504051552fd84ef3b0492961ff Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 12:37:44 -0800 Subject: [PATCH 02/12] Remove UserStore from Worktree --- crates/project/src/project.rs | 21 ++-------- crates/project/src/worktree.rs | 75 ++++++++-------------------------- 2 files changed, 20 insertions(+), 76 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 09925badf62653e9fdf84b13b1433c80fbdb18b4..b9ad5a02ea88891c16b31a4d84d7f67561072b79 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -230,17 +230,8 @@ impl Project { let mut worktrees = Vec::new(); for worktree in response.worktrees { - worktrees.push( - Worktree::remote( - remote_id, - replica_id, - worktree, - client.clone(), - user_store.clone(), - cx, - ) - .await?, - ); + worktrees + .push(Worktree::remote(remote_id, replica_id, worktree, client.clone(), cx).await?); } let user_ids = response @@ -891,11 +882,9 @@ impl Project { ) -> Task>> { let fs = self.fs.clone(); let client = self.client.clone(); - let user_store = self.user_store.clone(); let path = Arc::from(abs_path.as_ref()); cx.spawn(|project, mut cx| async move { - let worktree = - Worktree::open_local(client.clone(), user_store, path, weak, fs, &mut cx).await?; + let worktree = Worktree::open_local(client.clone(), path, weak, fs, &mut cx).await?; let (remote_project_id, is_shared) = project.update(&mut cx, |project, cx| { project.add_worktree(&worktree, cx); @@ -1100,12 +1089,10 @@ impl Project { .payload .worktree .ok_or_else(|| anyhow!("invalid worktree"))?; - let user_store = self.user_store.clone(); cx.spawn(|this, mut cx| { async move { let worktree = - Worktree::remote(remote_id, replica_id, worktree, client, user_store, &mut cx) - .await?; + Worktree::remote(remote_id, replica_id, worktree, client, &mut cx).await?; this.update(&mut cx, |this, cx| this.add_worktree(&worktree, cx)); Ok(()) } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index c07178e6d96c9a58da64ecf16c682760b873e417..19c78b7a6e494d5e660afa209dc1e751dc3005ca 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5,7 +5,7 @@ use super::{ }; use ::ignore::gitignore::{Gitignore, GitignoreBuilder}; use anyhow::{anyhow, Result}; -use client::{proto, Client, PeerId, TypedEnvelope, UserStore}; +use client::{proto, Client, PeerId, TypedEnvelope}; use clock::ReplicaId; use collections::{hash_map, HashMap, HashSet}; use futures::{Stream, StreamExt}; @@ -89,14 +89,12 @@ impl Entity for Worktree { impl Worktree { pub async fn open_local( client: Arc, - user_store: ModelHandle, path: impl Into>, weak: bool, fs: Arc, cx: &mut AsyncAppContext, ) -> Result> { - let (tree, scan_states_tx) = - LocalWorktree::new(client, user_store, path, weak, fs.clone(), cx).await?; + let (tree, scan_states_tx) = LocalWorktree::new(client, path, weak, fs.clone(), cx).await?; tree.update(cx, |tree, cx| { let tree = tree.as_local_mut().unwrap(); let abs_path = tree.snapshot.abs_path.clone(); @@ -117,7 +115,6 @@ impl Worktree { replica_id: ReplicaId, worktree: proto::Worktree, client: Arc, - user_store: ModelHandle, cx: &mut AsyncAppContext, ) -> Result> { let remote_id = worktree.id; @@ -225,7 +222,6 @@ impl Worktree { loading_buffers: Default::default(), open_buffers: Default::default(), queued_operations: Default::default(), - user_store, diagnostic_summaries, weak, }) @@ -304,13 +300,6 @@ impl Worktree { } } - pub fn user_store(&self) -> &ModelHandle { - match self { - Worktree::Local(worktree) => &worktree.user_store, - Worktree::Remote(worktree) => &worktree.user_store, - } - } - pub fn diagnostic_summaries<'a>( &'a self, ) -> impl Iterator, DiagnosticSummary)> + 'a { @@ -390,7 +379,7 @@ impl Worktree { }) } - #[cfg(feature = "test-support")] + #[cfg(any(test, feature = "test-support"))] pub(crate) fn has_open_buffer(&self, path: impl AsRef, cx: &AppContext) -> bool { let mut open_buffers: Box> = match self { Worktree::Local(worktree) => Box::new(worktree.open_buffers.values()), @@ -788,7 +777,6 @@ pub struct LocalWorktree { diagnostic_summaries: TreeMap, queued_operations: Vec<(u64, Operation)>, client: Arc, - user_store: ModelHandle, fs: Arc, weak: bool, } @@ -815,7 +803,6 @@ pub struct RemoteWorktree { replica_id: ReplicaId, loading_buffers: LoadingBuffers, open_buffers: HashMap, - user_store: ModelHandle, queued_operations: Vec<(u64, Operation)>, diagnostic_summaries: TreeMap, weak: bool, @@ -836,7 +823,6 @@ struct WorktreeConfig { impl LocalWorktree { async fn new( client: Arc, - user_store: ModelHandle, path: impl Into>, weak: bool, fs: Arc, @@ -904,7 +890,6 @@ impl LocalWorktree { diagnostic_summaries: Default::default(), queued_operations: Default::default(), client, - user_store, fs, weak, }; @@ -3075,7 +3060,7 @@ mod tests { use super::*; use crate::fs::FakeFs; use anyhow::Result; - use client::test::{FakeHttpClient, FakeServer}; + use client::test::FakeHttpClient; use fs::RealFs; use language::{Diagnostic, DiagnosticEntry}; use lsp::Url; @@ -3092,7 +3077,7 @@ mod tests { use util::test::temp_tree; #[gpui::test] - async fn test_traversal(mut cx: gpui::TestAppContext) { + async fn test_traversal(cx: gpui::TestAppContext) { let fs = FakeFs::new(); fs.insert_tree( "/root", @@ -3107,12 +3092,10 @@ mod tests { .await; let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); + let client = Client::new(http_client); let tree = Worktree::open_local( client, - user_store, Arc::from(Path::new("/root")), false, Arc::new(fs), @@ -3145,12 +3128,10 @@ mod tests { })); let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); + let client = Client::new(http_client); let tree = Worktree::open_local( client, - user_store, dir.path(), false, Arc::new(RealFs), @@ -3180,12 +3161,10 @@ mod tests { let file_path = dir.path().join("file1"); let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); + let client = Client::new(http_client); let tree = Worktree::open_local( client, - user_store, file_path.clone(), false, Arc::new(RealFs), @@ -3227,14 +3206,10 @@ mod tests { } })); - let user_id = 5; let http_client = FakeHttpClient::with_404_response(); - let mut client = Client::new(http_client.clone()); - let server = FakeServer::for_client(user_id, &mut client, &cx).await; - let user_store = server.build_user_store(client.clone(), &mut cx).await; + let client = Client::new(http_client.clone()); let tree = Worktree::open_local( client, - user_store.clone(), dir.path(), false, Arc::new(RealFs), @@ -3275,7 +3250,6 @@ mod tests { 1, initial_snapshot.to_proto(&Default::default(), Default::default()), Client::new(http_client.clone()), - user_store, &mut cx.to_async(), ) .await @@ -3367,7 +3341,7 @@ mod tests { } #[gpui::test] - async fn test_rescan_with_gitignore(mut cx: gpui::TestAppContext) { + async fn test_rescan_with_gitignore(cx: gpui::TestAppContext) { let dir = temp_tree(json!({ ".git": {}, ".gitignore": "ignored-dir\n", @@ -3381,11 +3355,9 @@ mod tests { let http_client = FakeHttpClient::with_404_response(); let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); let tree = Worktree::open_local( client, - user_store, dir.path(), false, Arc::new(RealFs), @@ -3420,11 +3392,8 @@ mod tests { #[gpui::test] async fn test_buffer_deduping(mut cx: gpui::TestAppContext) { - let user_id = 100; let http_client = FakeHttpClient::with_404_response(); - let mut client = Client::new(http_client); - let server = FakeServer::for_client(user_id, &mut client, &cx).await; - let user_store = server.build_user_store(client.clone(), &mut cx).await; + let client = Client::new(http_client); let fs = Arc::new(FakeFs::new()); fs.insert_tree( @@ -3436,16 +3405,10 @@ mod tests { ) .await; - let worktree = Worktree::open_local( - client.clone(), - user_store, - "/the-dir".as_ref(), - false, - fs, - &mut cx.to_async(), - ) - .await - .unwrap(); + let worktree = + Worktree::open_local(client, "/the-dir".as_ref(), false, fs, &mut cx.to_async()) + .await + .unwrap(); // Spawn multiple tasks to open paths, repeating some paths. let (buffer_a_1, buffer_b, buffer_a_2) = worktree.update(&mut cx, |worktree, cx| { @@ -3489,11 +3452,9 @@ mod tests { })); let http_client = FakeHttpClient::with_404_response(); let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); let tree = Worktree::open_local( client, - user_store, dir.path(), false, Arc::new(RealFs), @@ -3625,12 +3586,10 @@ mod tests { let initial_contents = "aaa\nbbbbb\nc\n"; let dir = temp_tree(json!({ "the-file": initial_contents })); let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); + let client = Client::new(http_client); let tree = Worktree::open_local( client, - user_store, dir.path(), false, Arc::new(RealFs), @@ -3726,7 +3685,6 @@ mod tests { let fs = Arc::new(FakeFs::new()); let http_client = FakeHttpClient::with_404_response(); let client = Client::new(http_client.clone()); - let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); fs.insert_tree( "/the-dir", @@ -3745,7 +3703,6 @@ mod tests { let worktree = Worktree::open_local( client.clone(), - user_store, "/the-dir".as_ref(), false, fs, From bd49a02c9255d4af1abf551313bb4b1d765f8f73 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 15:46:27 -0800 Subject: [PATCH 03/12] Move buffers from worktree to project Co-Authored-By: Nathan Sobo --- crates/diagnostics/src/diagnostics.rs | 13 +- crates/language/src/buffer.rs | 9 +- crates/project/src/project.rs | 1281 +++++++++++++++++++-- crates/project/src/worktree.rs | 1526 +------------------------ 4 files changed, 1265 insertions(+), 1564 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 4abea30800cfb6c5d0abb88f41c5aa5139791032..c1325c922e72a5c0931df8d7faa7af0c2654e2d2 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -764,9 +764,9 @@ mod tests { ) .await; - let (worktree, _) = project + let worktree = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path("/test", false, cx) + project.add_local_worktree("/test", false, cx) }) .await .unwrap(); @@ -777,9 +777,8 @@ mod tests { worktree .as_local_mut() .unwrap() - .update_diagnostic_entries( + .update_diagnostics( Arc::from("/test/main.rs".as_ref()), - None, vec![ DiagnosticEntry { range: PointUtf16::new(1, 8)..PointUtf16::new(1, 9), @@ -930,9 +929,8 @@ mod tests { worktree .as_local_mut() .unwrap() - .update_diagnostic_entries( + .update_diagnostics( Arc::from("/test/consts.rs".as_ref()), - None, vec![DiagnosticEntry { range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15), diagnostic: Diagnostic { @@ -1036,9 +1034,8 @@ mod tests { worktree .as_local_mut() .unwrap() - .update_diagnostic_entries( + .update_diagnostics( Arc::from("/test/consts.rs".as_ref()), - None, vec![ DiagnosticEntry { range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15), diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 042c0148449240261672d08ba24169a23bb570ac..0c97179e6f1ef949c965ed2ff267eec9a5e2c851 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -856,7 +856,7 @@ impl Buffer { version: Option, mut diagnostics: Vec>, cx: &mut ModelContext, - ) -> Result + ) -> Result<()> where T: Copy + Ord + TextDimension + Sub + Clip + ToPoint, { @@ -944,10 +944,13 @@ impl Buffer { let set = DiagnosticSet::new(sanitized_diagnostics, content); self.apply_diagnostic_update(set.clone(), cx); - Ok(Operation::UpdateDiagnostics { + + let op = Operation::UpdateDiagnostics { diagnostics: set.iter().cloned().collect(), lamport_timestamp: self.text.lamport_clock.tick(), - }) + }; + self.send_operation(op, cx); + Ok(()) } fn request_autoindent(&mut self, cx: &mut ModelContext) { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index b9ad5a02ea88891c16b31a4d84d7f67561072b79..efadbd707c1a466d2786f5fa04190ac47c43eb75 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -13,17 +13,22 @@ use gpui::{ WeakModelHandle, }; use language::{ - Bias, Buffer, DiagnosticEntry, File as _, Language, LanguageRegistry, ToOffset, ToPointUtf16, + range_from_lsp, Bias, Buffer, Diagnostic, DiagnosticEntry, File as _, Language, + LanguageRegistry, Operation, ToOffset, ToPointUtf16, }; use lsp::{DiagnosticSeverity, LanguageServer}; use postage::{prelude::Stream, watch}; use smol::block_on; use std::{ + convert::TryInto, ops::Range, path::{Path, PathBuf}, - sync::{atomic::AtomicBool, Arc}, + sync::{ + atomic::{AtomicBool, Ordering::SeqCst}, + Arc, + }, }; -use util::{ResultExt, TryFutureExt as _}; +use util::{post_inc, ResultExt, TryFutureExt as _}; pub use fs::*; pub use worktree::*; @@ -40,6 +45,19 @@ pub struct Project { collaborators: HashMap, subscriptions: Vec, language_servers_with_diagnostics_running: isize, + open_buffers: HashMap, + loading_buffers: HashMap< + ProjectPath, + postage::watch::Receiver< + Option, Arc), Arc>>, + >, + >, + shared_buffers: HashMap>>, +} + +enum OpenBuffer { + Operations(Vec), + Loaded(WeakModelHandle), } enum WorktreeHandle { @@ -192,6 +210,9 @@ impl Project { Self { worktrees: Default::default(), collaborators: Default::default(), + open_buffers: Default::default(), + loading_buffers: Default::default(), + shared_buffers: Default::default(), client_state: ProjectClientState::Local { is_shared: false, remote_id_tx, @@ -251,6 +272,9 @@ impl Project { Ok(cx.add_model(|cx| { let mut this = Self { worktrees: Vec::new(), + open_buffers: Default::default(), + loading_buffers: Default::default(), + shared_buffers: Default::default(), active_entry: None, collaborators, languages, @@ -474,11 +498,59 @@ impl Project { let worktree = if let Some(worktree) = self.worktree_for_id(path.worktree_id, cx) { worktree } else { - return cx.spawn(|_, _| async move { Err(anyhow!("no such worktree")) }); + return cx + .foreground() + .spawn(async move { Err(anyhow!("no such worktree")) }); + }; + + // If there is already a buffer for the given path, then return it. + let existing_buffer = self.get_open_buffer(&path, cx); + if let Some(existing_buffer) = existing_buffer { + return cx.foreground().spawn(async move { Ok(existing_buffer) }); + } + + let mut loading_watch = match self.loading_buffers.entry(path.clone()) { + // If the given path is already being loaded, then wait for that existing + // task to complete and return the same buffer. + hash_map::Entry::Occupied(e) => e.get().clone(), + + // Otherwise, record the fact that this path is now being loaded. + hash_map::Entry::Vacant(entry) => { + let (mut tx, rx) = postage::watch::channel(); + entry.insert(rx.clone()); + + let load_buffer = worktree.update(cx, |worktree, cx| match worktree { + Worktree::Local(worktree) => worktree.open_buffer(&path.path, cx), + Worktree::Remote(worktree) => worktree.open_buffer(&path.path, cx), + }); + + cx.spawn(move |this, mut cx| async move { + let load_result = load_buffer.await; + *tx.borrow_mut() = Some(this.update(&mut cx, |this, _| { + // Record the fact that the buffer is no longer loading. + this.loading_buffers.remove(&path); + let buffer = load_result.map_err(Arc::new)?; + this.open_buffers + .insert(buffer.id(), OpenBuffer::Loaded(buffer.downgrade())); + Ok((buffer, Arc::new(AtomicBool::new(true)))) + })); + }) + .detach(); + rx + } }; - let buffer_task = worktree.update(cx, |worktree, cx| worktree.open_buffer(path.path, cx)); + cx.spawn(|this, mut cx| async move { - let (buffer, buffer_is_new) = buffer_task.await?; + let (buffer, buffer_is_new) = loop { + if let Some(result) = loading_watch.borrow().as_ref() { + break match result { + Ok((buf, is_new)) => Ok((buf.clone(), is_new.fetch_and(false, SeqCst))), + Err(error) => Err(anyhow!("{}", error)), + }; + } + loading_watch.recv().await; + }?; + if buffer_is_new { this.update(&mut cx, |this, cx| { this.assign_language_to_buffer(worktree, buffer.clone(), cx) @@ -506,6 +578,8 @@ impl Project { }) .await?; this.update(&mut cx, |this, cx| { + this.open_buffers + .insert(buffer.id(), OpenBuffer::Loaded(buffer.downgrade())); this.assign_language_to_buffer(worktree, buffer, cx) }); Ok(()) @@ -515,8 +589,43 @@ impl Project { #[cfg(any(test, feature = "test-support"))] pub fn has_open_buffer(&self, path: impl Into, cx: &AppContext) -> bool { let path = path.into(); - self.worktree_for_id(path.worktree_id, cx) - .map_or(false, |tree| tree.read(cx).has_open_buffer(path.path, cx)) + if let Some(worktree) = self.worktree_for_id(path.worktree_id, cx) { + self.open_buffers.iter().any(|(_, buffer)| { + if let Some(buffer) = buffer.upgrade(cx) { + if let Some(file) = File::from_dyn(buffer.read(cx).file()) { + if file.worktree == worktree && file.path() == &path.path { + return true; + } + } + } + false + }) + } else { + false + } + } + + fn get_open_buffer( + &mut self, + path: &ProjectPath, + cx: &mut ModelContext, + ) -> Option> { + let mut result = None; + let worktree = self.worktree_for_id(path.worktree_id, cx)?; + self.open_buffers.retain(|_, buffer| { + if let OpenBuffer::Loaded(buffer) = buffer { + if let Some(buffer) = buffer.upgrade(cx) { + if let Some(file) = File::from_dyn(buffer.read(cx).file()) { + if file.worktree == worktree && file.path() == &path.path { + result = Some(buffer); + } + } + return true; + } + } + false + }); + result } fn assign_language_to_buffer( @@ -670,13 +779,13 @@ impl Project { Some(language_server) } - fn update_diagnostics( + pub fn update_diagnostics( &mut self, - diagnostics: lsp::PublishDiagnosticsParams, + params: lsp::PublishDiagnosticsParams, disk_based_sources: &HashSet, cx: &mut ModelContext, ) -> Result<()> { - let path = diagnostics + let path = params .uri .to_file_path() .map_err(|_| anyhow!("URI is not a file"))?; @@ -687,13 +796,113 @@ impl Project { worktree_id: worktree.read(cx).id(), path: relative_path.into(), }; + let mut next_group_id = 0; + let mut diagnostics = Vec::default(); + let mut primary_diagnostic_group_ids = HashMap::default(); + let mut sources_by_group_id = HashMap::default(); + let mut supporting_diagnostic_severities = HashMap::default(); + for diagnostic in ¶ms.diagnostics { + let source = diagnostic.source.as_ref(); + let code = diagnostic.code.as_ref().map(|code| match code { + lsp::NumberOrString::Number(code) => code.to_string(), + lsp::NumberOrString::String(code) => code.clone(), + }); + let range = range_from_lsp(diagnostic.range); + let is_supporting = diagnostic + .related_information + .as_ref() + .map_or(false, |infos| { + infos.iter().any(|info| { + primary_diagnostic_group_ids.contains_key(&( + source, + code.clone(), + range_from_lsp(info.location.range), + )) + }) + }); + + if is_supporting { + if let Some(severity) = diagnostic.severity { + supporting_diagnostic_severities + .insert((source, code.clone(), range), severity); + } + } else { + let group_id = post_inc(&mut next_group_id); + let is_disk_based = + source.map_or(false, |source| disk_based_sources.contains(source)); + + sources_by_group_id.insert(group_id, source); + primary_diagnostic_group_ids + .insert((source, code.clone(), range.clone()), group_id); + + diagnostics.push(DiagnosticEntry { + range, + diagnostic: Diagnostic { + code: code.clone(), + severity: diagnostic.severity.unwrap_or(DiagnosticSeverity::ERROR), + message: diagnostic.message.clone(), + group_id, + is_primary: true, + is_valid: true, + is_disk_based, + }, + }); + if let Some(infos) = &diagnostic.related_information { + for info in infos { + if info.location.uri == params.uri { + let range = range_from_lsp(info.location.range); + diagnostics.push(DiagnosticEntry { + range, + diagnostic: Diagnostic { + code: code.clone(), + severity: DiagnosticSeverity::INFORMATION, + message: info.message.clone(), + group_id, + is_primary: false, + is_valid: true, + is_disk_based, + }, + }); + } + } + } + } + } + + for entry in &mut diagnostics { + let diagnostic = &mut entry.diagnostic; + if !diagnostic.is_primary { + let source = *sources_by_group_id.get(&diagnostic.group_id).unwrap(); + if let Some(&severity) = supporting_diagnostic_severities.get(&( + source, + diagnostic.code.clone(), + entry.range.clone(), + )) { + diagnostic.severity = severity; + } + } + } + + for buffer in self.open_buffers.values() { + if let Some(buffer) = buffer.upgrade(cx) { + if buffer + .read(cx) + .file() + .map_or(false, |file| *file.path() == project_path.path) + { + buffer.update(cx, |buffer, cx| { + buffer.update_diagnostics(params.version, diagnostics.clone(), cx) + })?; + break; + } + } + } + worktree.update(cx, |worktree, cx| { - worktree.as_local_mut().unwrap().update_diagnostics( - project_path.path.clone(), - diagnostics, - disk_based_sources, - cx, - ) + worktree + .as_local_mut() + .ok_or_else(|| anyhow!("not a local worktree"))? + .update_diagnostics(project_path.path.clone(), diagnostics, cx) })?; cx.emit(Event::DiagnosticsUpdated(project_path)); Ok(()) @@ -874,7 +1083,7 @@ impl Project { } } - fn add_local_worktree( + pub fn add_local_worktree( &self, abs_path: impl AsRef, weak: bool, @@ -884,7 +1093,7 @@ impl Project { let client = self.client.clone(); let path = Arc::from(abs_path.as_ref()); cx.spawn(|project, mut cx| async move { - let worktree = Worktree::open_local(client.clone(), path, weak, fs, &mut cx).await?; + let worktree = Worktree::local(client.clone(), path, weak, fs, &mut cx).await?; let (remote_project_id, is_shared) = project.update(&mut cx, |project, cx| { project.add_worktree(&worktree, cx); @@ -921,6 +1130,10 @@ impl Project { fn add_worktree(&mut self, worktree: &ModelHandle, cx: &mut ModelContext) { cx.observe(&worktree, |_, _, cx| cx.notify()).detach(); + cx.subscribe(&worktree, |this, worktree, _, cx| { + this.update_open_buffers(worktree, cx) + }) + .detach(); let push_weak_handle = { let worktree = worktree.read(cx); @@ -942,6 +1155,74 @@ impl Project { cx.notify(); } + fn update_open_buffers( + &mut self, + worktree_handle: ModelHandle, + cx: &mut ModelContext, + ) { + let local = worktree_handle.read(cx).is_local(); + let snapshot = worktree_handle.read(cx).snapshot(); + let worktree_path = snapshot.abs_path(); + let mut buffers_to_delete = Vec::new(); + for (buffer_id, buffer) in &self.open_buffers { + if let OpenBuffer::Loaded(buffer) = buffer { + if let Some(buffer) = buffer.upgrade(cx) { + buffer.update(cx, |buffer, cx| { + if let Some(old_file) = File::from_dyn(buffer.file()) { + if old_file.worktree != worktree_handle { + return; + } + + let new_file = if let Some(entry) = old_file + .entry_id + .and_then(|entry_id| snapshot.entry_for_id(entry_id)) + { + File { + is_local: local, + worktree_path: worktree_path.clone(), + entry_id: Some(entry.id), + mtime: entry.mtime, + path: entry.path.clone(), + worktree: worktree_handle.clone(), + } + } else if let Some(entry) = + snapshot.entry_for_path(old_file.path().as_ref()) + { + File { + is_local: local, + worktree_path: worktree_path.clone(), + entry_id: Some(entry.id), + mtime: entry.mtime, + path: entry.path.clone(), + worktree: worktree_handle.clone(), + } + } else { + File { + is_local: local, + worktree_path: worktree_path.clone(), + entry_id: None, + path: old_file.path().clone(), + mtime: old_file.mtime(), + worktree: worktree_handle.clone(), + } + }; + + if let Some(task) = buffer.file_updated(Box::new(new_file), cx) { + task.detach(); + } + } + }); + } else { + buffers_to_delete.push(*buffer_id); + } + } + } + + for buffer_id in buffers_to_delete { + self.open_buffers.remove(&buffer_id); + } + } + pub fn set_active_path(&mut self, entry: Option, cx: &mut ModelContext) { let new_active_entry = entry.and_then(|project_path| { let worktree = self.worktree_for_id(project_path.worktree_id, cx)?; @@ -1069,11 +1350,15 @@ impl Project { .remove(&peer_id) .ok_or_else(|| anyhow!("unknown peer {:?}", peer_id))? .replica_id; - for worktree in self.worktrees(cx).collect::>() { - worktree.update(cx, |worktree, cx| { - worktree.remove_collaborator(peer_id, replica_id, cx); - }) + self.shared_buffers.remove(&peer_id); + for (_, buffer) in &self.open_buffers { + if let OpenBuffer::Loaded(buffer) = buffer { + if let Some(buffer) = buffer.upgrade(cx) { + buffer.update(cx, |buffer, cx| buffer.remove_peer(replica_id, cx)); + } + } } + cx.notify(); Ok(()) } @@ -1180,11 +1465,27 @@ impl Project { _: Arc, cx: &mut ModelContext, ) -> Result<()> { - let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(worktree) = self.worktree_for_id(worktree_id, cx) { - worktree.update(cx, |worktree, cx| { - worktree.handle_update_buffer(envelope, cx) - })?; + let payload = envelope.payload.clone(); + let buffer_id = payload.buffer_id as usize; + let ops = payload + .operations + .into_iter() + .map(|op| language::proto::deserialize_operation(op)) + .collect::, _>>()?; + match self.open_buffers.get_mut(&buffer_id) { + Some(OpenBuffer::Operations(pending_ops)) => pending_ops.extend(ops), + Some(OpenBuffer::Loaded(buffer)) => { + if let Some(buffer) = buffer.upgrade(cx) { + buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx))?; + } else { + self.open_buffers + .insert(buffer_id, OpenBuffer::Operations(ops)); + } + } + None => { + self.open_buffers + .insert(buffer_id, OpenBuffer::Operations(ops)); + } } Ok(()) } @@ -1195,12 +1496,42 @@ impl Project { rpc: Arc, cx: &mut ModelContext, ) -> Result<()> { - let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(worktree) = self.worktree_for_id(worktree_id, cx) { - worktree.update(cx, |worktree, cx| { - worktree.handle_save_buffer(envelope, rpc, cx) - })?; - } + let sender_id = envelope.original_sender_id()?; + let project_id = self.remote_id().ok_or_else(|| anyhow!("not connected"))?; + let buffer = self + .shared_buffers + .get(&sender_id) + .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned()) + .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?; + let receipt = envelope.receipt(); + let worktree_id = envelope.payload.worktree_id; + let buffer_id = envelope.payload.buffer_id; + let save = cx.spawn(|_, mut cx| async move { + buffer.update(&mut cx, |buffer, cx| buffer.save(cx)).await + }); + + cx.background() + .spawn( + async move { + let (version, mtime) = save.await?; + + rpc.respond( + receipt, + proto::BufferSaved { + project_id, + worktree_id, + buffer_id, + version: (&version).into(), + mtime: Some(mtime.into()), + }, + ) + .await?; + + Ok(()) + } + .log_err(), + ) + .detach(); Ok(()) } @@ -1210,12 +1541,36 @@ impl Project { rpc: Arc, cx: &mut ModelContext, ) -> Result<()> { - let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(worktree) = self.worktree_for_id(worktree_id, cx) { - worktree.update(cx, |worktree, cx| { - worktree.handle_format_buffer(envelope, rpc, cx) - })?; - } + let receipt = envelope.receipt(); + let sender_id = envelope.original_sender_id()?; + let buffer = self + .shared_buffers + .get(&sender_id) + .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned()) + .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?; + cx.spawn(|_, mut cx| async move { + let format = buffer.update(&mut cx, |buffer, cx| buffer.format(cx)).await; + // We spawn here in order to enqueue the sending of `Ack` *after* transmission of edits + // associated with formatting. + cx.spawn(|_| async move { + match format { + Ok(()) => rpc.respond(receipt, proto::Ack {}).await?, + Err(error) => { + rpc.respond_with_error( + receipt, + proto::Error { + message: error.to_string(), + }, + ) + .await? + } + } + Ok::<_, anyhow::Error>(()) + }) + .await + .log_err(); + }) + .detach(); Ok(()) } @@ -1228,28 +1583,30 @@ impl Project { let receipt = envelope.receipt(); let peer_id = envelope.original_sender_id()?; let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - let worktree = self - .worktree_for_id(worktree_id, cx) - .ok_or_else(|| anyhow!("no such worktree"))?; - - let task = self.open_buffer( + let open_buffer = self.open_buffer( ProjectPath { worktree_id, path: PathBuf::from(envelope.payload.path).into(), }, cx, ); - cx.spawn(|_, mut cx| { + cx.spawn(|this, mut cx| { async move { - let buffer = task.await?; - let response = worktree.update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .open_remote_buffer(peer_id, buffer, cx) + let buffer = open_buffer.await?; + this.update(&mut cx, |this, _| { + this.shared_buffers + .entry(peer_id) + .or_default() + .insert(buffer.id() as u64, buffer.clone()); }); - rpc.respond(receipt, response).await?; - Ok(()) + let message = buffer.read_with(&cx, |buffer, _| buffer.to_proto()); + rpc.respond( + receipt, + proto::OpenBufferResponse { + buffer: Some(message), + }, + ) + .await } .log_err() }) @@ -1263,14 +1620,9 @@ impl Project { _: Arc, cx: &mut ModelContext, ) -> anyhow::Result<()> { - let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(worktree) = self.worktree_for_id(worktree_id, cx) { - worktree.update(cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .close_remote_buffer(envelope, cx) - })?; + if let Some(shared_buffers) = self.shared_buffers.get_mut(&envelope.original_sender_id()?) { + shared_buffers.remove(&envelope.payload.buffer_id); + cx.notify(); } Ok(()) } @@ -1281,10 +1633,26 @@ impl Project { _: Arc, cx: &mut ModelContext, ) -> Result<()> { - let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id); - if let Some(worktree) = self.worktree_for_id(worktree_id, cx) { - worktree.update(cx, |worktree, cx| { - worktree.handle_buffer_saved(envelope, cx) + let payload = envelope.payload.clone(); + let buffer = self + .open_buffers + .get(&(payload.buffer_id as usize)) + .and_then(|buf| { + if let OpenBuffer::Loaded(buffer) = buf { + buffer.upgrade(cx) + } else { + None + } + }); + if let Some(buffer) = buffer { + buffer.update(cx, |buffer, cx| { + let version = payload.version.try_into()?; + let mtime = payload + .mtime + .ok_or_else(|| anyhow!("missing mtime"))? + .into(); + buffer.did_save(version, mtime, None, cx); + Result::<_, anyhow::Error>::Ok(()) })?; } Ok(()) @@ -1474,6 +1842,15 @@ impl> From<(WorktreeId, P)> for ProjectPath { } } +impl OpenBuffer { + fn upgrade(&self, cx: &AppContext) -> Option> { + match self { + OpenBuffer::Loaded(buffer) => buffer.upgrade(cx), + OpenBuffer::Operations(_) => None, + } + } +} + #[cfg(test)] mod tests { use super::{Event, *}; @@ -1487,8 +1864,10 @@ mod tests { }; use lsp::Url; use serde_json::json; - use std::{os::unix, path::PathBuf}; + use std::{cell::RefCell, os::unix, path::PathBuf, rc::Rc}; + use unindent::Unindent as _; use util::test::temp_tree; + use worktree::WorktreeHandle as _; #[gpui::test] async fn test_populate_and_search(mut cx: gpui::TestAppContext) { @@ -1515,7 +1894,7 @@ mod tests { ) .unwrap(); - let project = build_project(&mut cx); + let project = build_project(Arc::new(RealFs), &mut cx); let (tree, _) = project .update(&mut cx, |project, cx| { @@ -1660,8 +2039,8 @@ mod tests { Event::DiskBasedDiagnosticsFinished ); - let (buffer, _) = tree - .update(&mut cx, |tree, cx| tree.open_buffer("a.rs", cx)) + let buffer = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "a.rs"), cx)) .await .unwrap(); @@ -1697,7 +2076,7 @@ mod tests { } })); - let project = build_project(&mut cx); + let project = build_project(Arc::new(RealFs), &mut cx); let (tree, _) = project .update(&mut cx, |project, cx| { project.find_or_create_worktree_for_abs_path(&dir.path(), false, cx) @@ -1838,9 +2217,765 @@ mod tests { } } - fn build_project(cx: &mut TestAppContext) -> ModelHandle { + #[gpui::test] + async fn test_save_file(mut cx: gpui::TestAppContext) { + let fs = Arc::new(FakeFs::new()); + fs.insert_tree( + "/dir", + json!({ + "file1": "the old contents", + }), + ) + .await; + + let project = build_project(fs.clone(), &mut cx); + let worktree_id = project + .update(&mut cx, |p, cx| p.add_local_worktree("/dir", false, cx)) + .await + .unwrap() + .read_with(&cx, |tree, _| tree.id()); + + let buffer = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "file1"), cx)) + .await + .unwrap(); + buffer + .update(&mut cx, |buffer, cx| { + assert_eq!(buffer.text(), "the old contents"); + buffer.edit(Some(0..0), "a line of text.\n".repeat(10 * 1024), cx); + buffer.save(cx) + }) + .await + .unwrap(); + + let new_text = fs.load(Path::new("/dir/file1")).await.unwrap(); + assert_eq!(new_text, buffer.read_with(&cx, |buffer, _| buffer.text())); + } + + #[gpui::test] + async fn test_save_in_single_file_worktree(mut cx: gpui::TestAppContext) { + let fs = Arc::new(FakeFs::new()); + fs.insert_tree( + "/dir", + json!({ + "file1": "the old contents", + }), + ) + .await; + + let project = build_project(fs.clone(), &mut cx); + let worktree_id = project + .update(&mut cx, |p, cx| { + p.add_local_worktree("/dir/file1", false, cx) + }) + .await + .unwrap() + .read_with(&cx, |tree, _| tree.id()); + + let buffer = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, ""), cx)) + .await + .unwrap(); + buffer + .update(&mut cx, |buffer, cx| { + buffer.edit(Some(0..0), "a line of text.\n".repeat(10 * 1024), cx); + buffer.save(cx) + }) + .await + .unwrap(); + + let new_text = fs.load(Path::new("/dir/file1")).await.unwrap(); + assert_eq!(new_text, buffer.read_with(&cx, |buffer, _| buffer.text())); + } + + #[gpui::test] + async fn test_rescan_and_remote_updates(mut cx: gpui::TestAppContext) { + let dir = temp_tree(json!({ + "a": { + "file1": "", + "file2": "", + "file3": "", + }, + "b": { + "c": { + "file4": "", + "file5": "", + } + } + })); + + let project = build_project(Arc::new(RealFs), &mut cx); + let rpc = project.read_with(&cx, |p, _| p.client.clone()); + + let tree = project + .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + .await + .unwrap(); + let worktree_id = tree.read_with(&cx, |tree, _| tree.id()); + + let buffer_for_path = |path: &'static str, cx: &mut gpui::TestAppContext| { + let buffer = project.update(cx, |p, cx| p.open_buffer((worktree_id, path), cx)); + async move { buffer.await.unwrap() } + }; + let id_for_path = |path: &'static str, cx: &gpui::TestAppContext| { + tree.read_with(cx, |tree, _| { + tree.entry_for_path(path) + .expect(&format!("no entry for path {}", path)) + .id + }) + }; + + let buffer2 = buffer_for_path("a/file2", &mut cx).await; + let buffer3 = buffer_for_path("a/file3", &mut cx).await; + let buffer4 = buffer_for_path("b/c/file4", &mut cx).await; + let buffer5 = buffer_for_path("b/c/file5", &mut cx).await; + + let file2_id = id_for_path("a/file2", &cx); + let file3_id = id_for_path("a/file3", &cx); + let file4_id = id_for_path("b/c/file4", &cx); + + // Wait for the initial scan. + cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) + .await; + + // Create a remote copy of this worktree. + let initial_snapshot = tree.read_with(&cx, |tree, _| tree.snapshot()); + let remote = Worktree::remote( + 1, + 1, + initial_snapshot.to_proto(&Default::default(), Default::default()), + rpc.clone(), + &mut cx.to_async(), + ) + .await + .unwrap(); + + cx.read(|cx| { + assert!(!buffer2.read(cx).is_dirty()); + assert!(!buffer3.read(cx).is_dirty()); + assert!(!buffer4.read(cx).is_dirty()); + assert!(!buffer5.read(cx).is_dirty()); + }); + + // Rename and delete files and directories. + tree.flush_fs_events(&cx).await; + std::fs::rename(dir.path().join("a/file3"), dir.path().join("b/c/file3")).unwrap(); + std::fs::remove_file(dir.path().join("b/c/file5")).unwrap(); + std::fs::rename(dir.path().join("b/c"), dir.path().join("d")).unwrap(); + std::fs::rename(dir.path().join("a/file2"), dir.path().join("a/file2.new")).unwrap(); + tree.flush_fs_events(&cx).await; + + let expected_paths = vec![ + "a", + "a/file1", + "a/file2.new", + "b", + "d", + "d/file3", + "d/file4", + ]; + + cx.read(|app| { + assert_eq!( + tree.read(app) + .paths() + .map(|p| p.to_str().unwrap()) + .collect::>(), + expected_paths + ); + + assert_eq!(id_for_path("a/file2.new", &cx), file2_id); + assert_eq!(id_for_path("d/file3", &cx), file3_id); + assert_eq!(id_for_path("d/file4", &cx), file4_id); + + assert_eq!( + buffer2.read(app).file().unwrap().path().as_ref(), + Path::new("a/file2.new") + ); + assert_eq!( + buffer3.read(app).file().unwrap().path().as_ref(), + Path::new("d/file3") + ); + assert_eq!( + buffer4.read(app).file().unwrap().path().as_ref(), + Path::new("d/file4") + ); + assert_eq!( + buffer5.read(app).file().unwrap().path().as_ref(), + Path::new("b/c/file5") + ); + + assert!(!buffer2.read(app).file().unwrap().is_deleted()); + assert!(!buffer3.read(app).file().unwrap().is_deleted()); + assert!(!buffer4.read(app).file().unwrap().is_deleted()); + assert!(buffer5.read(app).file().unwrap().is_deleted()); + }); + + // Update the remote worktree. Check that it becomes consistent with the + // local worktree. + remote.update(&mut cx, |remote, cx| { + let update_message = + tree.read(cx) + .snapshot() + .build_update(&initial_snapshot, 1, 1, true); + remote + .as_remote_mut() + .unwrap() + .snapshot + .apply_update(update_message) + .unwrap(); + + assert_eq!( + remote + .paths() + .map(|p| p.to_str().unwrap()) + .collect::>(), + expected_paths + ); + }); + } + + #[gpui::test] + async fn test_buffer_deduping(mut cx: gpui::TestAppContext) { + let fs = Arc::new(FakeFs::new()); + fs.insert_tree( + "/the-dir", + json!({ + "a.txt": "a-contents", + "b.txt": "b-contents", + }), + ) + .await; + + let project = build_project(fs.clone(), &mut cx); + let worktree_id = project + .update(&mut cx, |p, cx| p.add_local_worktree("/the-dir", false, cx)) + .await + .unwrap() + .read_with(&cx, |tree, _| tree.id()); + + // Spawn multiple tasks to open paths, repeating some paths. + let (buffer_a_1, buffer_b, buffer_a_2) = project.update(&mut cx, |p, cx| { + ( + p.open_buffer((worktree_id, "a.txt"), cx), + p.open_buffer((worktree_id, "b.txt"), cx), + p.open_buffer((worktree_id, "a.txt"), cx), + ) + }); + + let buffer_a_1 = buffer_a_1.await.unwrap(); + let buffer_a_2 = buffer_a_2.await.unwrap(); + let buffer_b = buffer_b.await.unwrap(); + assert_eq!(buffer_a_1.read_with(&cx, |b, _| b.text()), "a-contents"); + assert_eq!(buffer_b.read_with(&cx, |b, _| b.text()), "b-contents"); + + // There is only one buffer per path. + let buffer_a_id = buffer_a_1.id(); + assert_eq!(buffer_a_2.id(), buffer_a_id); + + // Open the same path again while it is still open. + drop(buffer_a_1); + let buffer_a_3 = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) + .await + .unwrap(); + + // There's still only one buffer per path. + assert_eq!(buffer_a_3.id(), buffer_a_id); + } + + #[gpui::test] + async fn test_buffer_is_dirty(mut cx: gpui::TestAppContext) { + use std::fs; + + let dir = temp_tree(json!({ + "file1": "abc", + "file2": "def", + "file3": "ghi", + })); + + let project = build_project(Arc::new(RealFs), &mut cx); + let worktree = project + .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + .await + .unwrap(); + let worktree_id = worktree.read_with(&cx, |worktree, _| worktree.id()); + + worktree.flush_fs_events(&cx).await; + worktree + .read_with(&cx, |t, _| t.as_local().unwrap().scan_complete()) + .await; + + let buffer1 = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "file1"), cx)) + .await + .unwrap(); + let events = Rc::new(RefCell::new(Vec::new())); + + // initially, the buffer isn't dirty. + buffer1.update(&mut cx, |buffer, cx| { + cx.subscribe(&buffer1, { + let events = events.clone(); + move |_, _, event, _| events.borrow_mut().push(event.clone()) + }) + .detach(); + + assert!(!buffer.is_dirty()); + assert!(events.borrow().is_empty()); + + buffer.edit(vec![1..2], "", cx); + }); + + // after the first edit, the buffer is dirty, and emits a dirtied event. + buffer1.update(&mut cx, |buffer, cx| { + assert!(buffer.text() == "ac"); + assert!(buffer.is_dirty()); + assert_eq!( + *events.borrow(), + &[language::Event::Edited, language::Event::Dirtied] + ); + events.borrow_mut().clear(); + buffer.did_save(buffer.version(), buffer.file().unwrap().mtime(), None, cx); + }); + + // after saving, the buffer is not dirty, and emits a saved event. + buffer1.update(&mut cx, |buffer, cx| { + assert!(!buffer.is_dirty()); + assert_eq!(*events.borrow(), &[language::Event::Saved]); + events.borrow_mut().clear(); + + buffer.edit(vec![1..1], "B", cx); + buffer.edit(vec![2..2], "D", cx); + }); + + // after editing again, the buffer is dirty, and emits another dirty event. + buffer1.update(&mut cx, |buffer, cx| { + assert!(buffer.text() == "aBDc"); + assert!(buffer.is_dirty()); + assert_eq!( + *events.borrow(), + &[ + language::Event::Edited, + language::Event::Dirtied, + language::Event::Edited, + ], + ); + events.borrow_mut().clear(); + + // TODO - currently, after restoring the buffer to its + // previously-saved state, the is still considered dirty. + buffer.edit([1..3], "", cx); + assert!(buffer.text() == "ac"); + assert!(buffer.is_dirty()); + }); + + assert_eq!(*events.borrow(), &[language::Event::Edited]); + + // When a file is deleted, the buffer is considered dirty. + let events = Rc::new(RefCell::new(Vec::new())); + let buffer2 = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "file2"), cx)) + .await + .unwrap(); + buffer2.update(&mut cx, |_, cx| { + cx.subscribe(&buffer2, { + let events = events.clone(); + move |_, _, event, _| events.borrow_mut().push(event.clone()) + }) + .detach(); + }); + + fs::remove_file(dir.path().join("file2")).unwrap(); + buffer2.condition(&cx, |b, _| b.is_dirty()).await; + assert_eq!( + *events.borrow(), + &[language::Event::Dirtied, language::Event::FileHandleChanged] + ); + + // When a file is already dirty when deleted, we don't emit a Dirtied event. + let events = Rc::new(RefCell::new(Vec::new())); + let buffer3 = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "file3"), cx)) + .await + .unwrap(); + buffer3.update(&mut cx, |_, cx| { + cx.subscribe(&buffer3, { + let events = events.clone(); + move |_, _, event, _| events.borrow_mut().push(event.clone()) + }) + .detach(); + }); + + worktree.flush_fs_events(&cx).await; + buffer3.update(&mut cx, |buffer, cx| { + buffer.edit(Some(0..0), "x", cx); + }); + events.borrow_mut().clear(); + fs::remove_file(dir.path().join("file3")).unwrap(); + buffer3 + .condition(&cx, |_, _| !events.borrow().is_empty()) + .await; + assert_eq!(*events.borrow(), &[language::Event::FileHandleChanged]); + cx.read(|cx| assert!(buffer3.read(cx).is_dirty())); + } + + #[gpui::test] + async fn test_buffer_file_changes_on_disk(mut cx: gpui::TestAppContext) { + use std::fs; + + let initial_contents = "aaa\nbbbbb\nc\n"; + let dir = temp_tree(json!({ "the-file": initial_contents })); + + let project = build_project(Arc::new(RealFs), &mut cx); + let worktree = project + .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + .await + .unwrap(); + let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); + + worktree + .read_with(&cx, |t, _| t.as_local().unwrap().scan_complete()) + .await; + + let abs_path = dir.path().join("the-file"); + let buffer = project + .update(&mut cx, |p, cx| { + p.open_buffer((worktree_id, "the-file"), cx) + }) + .await + .unwrap(); + + // TODO + // Add a cursor on each row. + // let selection_set_id = buffer.update(&mut cx, |buffer, cx| { + // assert!(!buffer.is_dirty()); + // buffer.add_selection_set( + // &(0..3) + // .map(|row| Selection { + // id: row as usize, + // start: Point::new(row, 1), + // end: Point::new(row, 1), + // reversed: false, + // goal: SelectionGoal::None, + // }) + // .collect::>(), + // cx, + // ) + // }); + + // Change the file on disk, adding two new lines of text, and removing + // one line. + buffer.read_with(&cx, |buffer, _| { + assert!(!buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + let new_contents = "AAAA\naaa\nBB\nbbbbb\n"; + fs::write(&abs_path, new_contents).unwrap(); + + // Because the buffer was not modified, it is reloaded from disk. Its + // contents are edited according to the diff between the old and new + // file contents. + buffer + .condition(&cx, |buffer, _| buffer.text() == new_contents) + .await; + + buffer.update(&mut cx, |buffer, _| { + assert_eq!(buffer.text(), new_contents); + assert!(!buffer.is_dirty()); + assert!(!buffer.has_conflict()); + + // TODO + // let cursor_positions = buffer + // .selection_set(selection_set_id) + // .unwrap() + // .selections::(&*buffer) + // .map(|selection| { + // assert_eq!(selection.start, selection.end); + // selection.start + // }) + // .collect::>(); + // assert_eq!( + // cursor_positions, + // [Point::new(1, 1), Point::new(3, 1), Point::new(4, 0)] + // ); + }); + + // Modify the buffer + buffer.update(&mut cx, |buffer, cx| { + buffer.edit(vec![0..0], " ", cx); + assert!(buffer.is_dirty()); + assert!(!buffer.has_conflict()); + }); + + // Change the file on disk again, adding blank lines to the beginning. + fs::write(&abs_path, "\n\n\nAAAA\naaa\nBB\nbbbbb\n").unwrap(); + + // Because the buffer is modified, it doesn't reload from disk, but is + // marked as having a conflict. + buffer + .condition(&cx, |buffer, _| buffer.has_conflict()) + .await; + } + + #[gpui::test] + async fn test_grouped_diagnostics(mut cx: gpui::TestAppContext) { + let fs = Arc::new(FakeFs::new()); + fs.insert_tree( + "/the-dir", + json!({ + "a.rs": " + fn foo(mut v: Vec) { + for x in &v { + v.push(1); + } + } + " + .unindent(), + }), + ) + .await; + + let project = build_project(fs.clone(), &mut cx); + let worktree = project + .update(&mut cx, |p, cx| p.add_local_worktree("/the-dir", false, cx)) + .await + .unwrap(); + let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); + + let buffer = project + .update(&mut cx, |p, cx| p.open_buffer((worktree_id, "a.rs"), cx)) + .await + .unwrap(); + + let buffer_uri = Url::from_file_path("/the-dir/a.rs").unwrap(); + let message = lsp::PublishDiagnosticsParams { + uri: buffer_uri.clone(), + diagnostics: vec![ + lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9)), + severity: Some(DiagnosticSeverity::WARNING), + message: "error 1".to_string(), + related_information: Some(vec![lsp::DiagnosticRelatedInformation { + location: lsp::Location { + uri: buffer_uri.clone(), + range: lsp::Range::new( + lsp::Position::new(1, 8), + lsp::Position::new(1, 9), + ), + }, + message: "error 1 hint 1".to_string(), + }]), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9)), + severity: Some(DiagnosticSeverity::HINT), + message: "error 1 hint 1".to_string(), + related_information: Some(vec![lsp::DiagnosticRelatedInformation { + location: lsp::Location { + uri: buffer_uri.clone(), + range: lsp::Range::new( + lsp::Position::new(1, 8), + lsp::Position::new(1, 9), + ), + }, + message: "original diagnostic".to_string(), + }]), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 17)), + severity: Some(DiagnosticSeverity::ERROR), + message: "error 2".to_string(), + related_information: Some(vec![ + lsp::DiagnosticRelatedInformation { + location: lsp::Location { + uri: buffer_uri.clone(), + range: lsp::Range::new( + lsp::Position::new(1, 13), + lsp::Position::new(1, 15), + ), + }, + message: "error 2 hint 1".to_string(), + }, + lsp::DiagnosticRelatedInformation { + location: lsp::Location { + uri: buffer_uri.clone(), + range: lsp::Range::new( + lsp::Position::new(1, 13), + lsp::Position::new(1, 15), + ), + }, + message: "error 2 hint 2".to_string(), + }, + ]), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(1, 13), lsp::Position::new(1, 15)), + severity: Some(DiagnosticSeverity::HINT), + message: "error 2 hint 1".to_string(), + related_information: Some(vec![lsp::DiagnosticRelatedInformation { + location: lsp::Location { + uri: buffer_uri.clone(), + range: lsp::Range::new( + lsp::Position::new(2, 8), + lsp::Position::new(2, 17), + ), + }, + message: "original diagnostic".to_string(), + }]), + ..Default::default() + }, + lsp::Diagnostic { + range: lsp::Range::new(lsp::Position::new(1, 13), lsp::Position::new(1, 15)), + severity: Some(DiagnosticSeverity::HINT), + message: "error 2 hint 2".to_string(), + related_information: Some(vec![lsp::DiagnosticRelatedInformation { + location: lsp::Location { + uri: buffer_uri.clone(), + range: lsp::Range::new( + lsp::Position::new(2, 8), + lsp::Position::new(2, 17), + ), + }, + message: "original diagnostic".to_string(), + }]), + ..Default::default() + }, + ], + version: None, + }; + + project + .update(&mut cx, |p, cx| { + p.update_diagnostics(message, &Default::default(), cx) + }) + .unwrap(); + let buffer = buffer.read_with(&cx, |buffer, _| buffer.snapshot()); + + assert_eq!( + buffer + .diagnostics_in_range::<_, Point>(0..buffer.len()) + .collect::>(), + &[ + DiagnosticEntry { + range: Point::new(1, 8)..Point::new(1, 9), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::WARNING, + message: "error 1".to_string(), + group_id: 0, + is_primary: true, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(1, 8)..Point::new(1, 9), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::HINT, + message: "error 1 hint 1".to_string(), + group_id: 0, + is_primary: false, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(1, 13)..Point::new(1, 15), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::HINT, + message: "error 2 hint 1".to_string(), + group_id: 1, + is_primary: false, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(1, 13)..Point::new(1, 15), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::HINT, + message: "error 2 hint 2".to_string(), + group_id: 1, + is_primary: false, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(2, 8)..Point::new(2, 17), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::ERROR, + message: "error 2".to_string(), + group_id: 1, + is_primary: true, + ..Default::default() + } + } + ] + ); + + assert_eq!( + buffer.diagnostic_group::(0).collect::>(), + &[ + DiagnosticEntry { + range: Point::new(1, 8)..Point::new(1, 9), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::WARNING, + message: "error 1".to_string(), + group_id: 0, + is_primary: true, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(1, 8)..Point::new(1, 9), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::HINT, + message: "error 1 hint 1".to_string(), + group_id: 0, + is_primary: false, + ..Default::default() + } + }, + ] + ); + assert_eq!( + buffer.diagnostic_group::(1).collect::>(), + &[ + DiagnosticEntry { + range: Point::new(1, 13)..Point::new(1, 15), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::HINT, + message: "error 2 hint 1".to_string(), + group_id: 1, + is_primary: false, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(1, 13)..Point::new(1, 15), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::HINT, + message: "error 2 hint 2".to_string(), + group_id: 1, + is_primary: false, + ..Default::default() + } + }, + DiagnosticEntry { + range: Point::new(2, 8)..Point::new(2, 17), + diagnostic: Diagnostic { + severity: DiagnosticSeverity::ERROR, + message: "error 2".to_string(), + group_id: 1, + is_primary: true, + ..Default::default() + } + } + ] + ); + } + + fn build_project(fs: Arc, cx: &mut TestAppContext) -> ModelHandle { let languages = Arc::new(LanguageRegistry::new()); - let fs = Arc::new(RealFs); let http_client = FakeHttpClient::with_404_response(); let client = client::Client::new(http_client.clone()); let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http_client, cx)); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 19c78b7a6e494d5e660afa209dc1e751dc3005ca..5bdc2221c16f7bd477664fb699e6ca86983f4de9 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -5,19 +5,16 @@ use super::{ }; use ::ignore::gitignore::{Gitignore, GitignoreBuilder}; use anyhow::{anyhow, Result}; -use client::{proto, Client, PeerId, TypedEnvelope}; +use client::{proto, Client, TypedEnvelope}; use clock::ReplicaId; -use collections::{hash_map, HashMap, HashSet}; +use collections::HashMap; use futures::{Stream, StreamExt}; use fuzzy::CharBag; use gpui::{ executor, AppContext, AsyncAppContext, Entity, ModelContext, ModelHandle, MutableAppContext, - Task, UpgradeModelHandle, WeakModelHandle, -}; -use language::{ - range_from_lsp, Buffer, Diagnostic, DiagnosticEntry, DiagnosticSeverity, File as _, Operation, - PointUtf16, Rope, + Task, }; +use language::{Buffer, DiagnosticEntry, Operation, PointUtf16, Rope}; use lazy_static::lazy_static; use parking_lot::Mutex; use postage::{ @@ -36,14 +33,14 @@ use std::{ ops::Deref, path::{Path, PathBuf}, sync::{ - atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst}, + atomic::{AtomicUsize, Ordering::SeqCst}, Arc, }, time::{Duration, SystemTime}, }; use sum_tree::{Bias, TreeMap}; use sum_tree::{Edit, SeekTarget, SumTree}; -use util::{post_inc, ResultExt, TryFutureExt}; +use util::ResultExt; lazy_static! { static ref GITIGNORE: &'static OsStr = OsStr::new(".gitignore"); @@ -64,8 +61,12 @@ pub enum Worktree { Remote(RemoteWorktree), } +pub enum Event { + UpdatedEntries, +} + impl Entity for Worktree { - type Event = (); + type Event = Event; fn release(&mut self, cx: &mut MutableAppContext) { if let Some(worktree) = self.as_local_mut() { @@ -87,7 +88,7 @@ impl Entity for Worktree { } impl Worktree { - pub async fn open_local( + pub async fn local( client: Arc, path: impl Into>, weak: bool, @@ -219,8 +220,6 @@ impl Worktree { snapshot_rx, updates_tx, client: client.clone(), - loading_buffers: Default::default(), - open_buffers: Default::default(), queued_operations: Default::default(), diagnostic_summaries, weak, @@ -288,18 +287,6 @@ impl Worktree { } } - pub fn remove_collaborator( - &mut self, - peer_id: PeerId, - replica_id: ReplicaId, - cx: &mut ModelContext, - ) { - match self { - Worktree::Local(worktree) => worktree.remove_collaborator(peer_id, replica_id, cx), - Worktree::Remote(worktree) => worktree.remove_collaborator(replica_id, cx), - } - } - pub fn diagnostic_summaries<'a>( &'a self, ) -> impl Iterator, DiagnosticSummary)> + 'a { @@ -311,267 +298,6 @@ impl Worktree { .map(|(path, summary)| (path.0.clone(), summary.clone())) } - pub(crate) fn loading_buffers<'a>(&'a mut self) -> &'a mut LoadingBuffers { - match self { - Worktree::Local(worktree) => &mut worktree.loading_buffers, - Worktree::Remote(worktree) => &mut worktree.loading_buffers, - } - } - - pub(crate) fn open_buffer( - &mut self, - path: impl AsRef, - cx: &mut ModelContext, - ) -> Task, bool)>> { - let path = path.as_ref(); - - // If there is already a buffer for the given path, then return it. - let existing_buffer = match self { - Worktree::Local(worktree) => worktree.get_open_buffer(path, cx), - Worktree::Remote(worktree) => worktree.get_open_buffer(path, cx), - }; - if let Some(existing_buffer) = existing_buffer { - return cx.spawn(move |_, _| async move { Ok((existing_buffer, false)) }); - } - - let is_new = Arc::new(AtomicBool::new(true)); - let path: Arc = Arc::from(path); - let mut loading_watch = match self.loading_buffers().entry(path.clone()) { - // If the given path is already being loaded, then wait for that existing - // task to complete and return the same buffer. - hash_map::Entry::Occupied(e) => e.get().clone(), - - // Otherwise, record the fact that this path is now being loaded. - hash_map::Entry::Vacant(entry) => { - let (mut tx, rx) = postage::watch::channel(); - entry.insert(rx.clone()); - - let load_buffer = match self { - Worktree::Local(worktree) => worktree.open_buffer(&path, cx), - Worktree::Remote(worktree) => worktree.open_buffer(&path, cx), - }; - cx.spawn(move |this, mut cx| async move { - let result = load_buffer.await; - - // After the buffer loads, record the fact that it is no longer - // loading. - this.update(&mut cx, |this, _| this.loading_buffers().remove(&path)); - *tx.borrow_mut() = Some(match result { - Ok(buffer) => Ok((buffer, is_new)), - Err(error) => Err(Arc::new(error)), - }); - }) - .detach(); - rx - } - }; - - cx.spawn(|_, _| async move { - loop { - if let Some(result) = loading_watch.borrow().as_ref() { - return match result { - Ok((buf, is_new)) => Ok((buf.clone(), is_new.fetch_and(false, SeqCst))), - Err(error) => Err(anyhow!("{}", error)), - }; - } - loading_watch.recv().await; - } - }) - } - - #[cfg(any(test, feature = "test-support"))] - pub(crate) fn has_open_buffer(&self, path: impl AsRef, cx: &AppContext) -> bool { - let mut open_buffers: Box> = match self { - Worktree::Local(worktree) => Box::new(worktree.open_buffers.values()), - Worktree::Remote(worktree) => { - Box::new(worktree.open_buffers.values().filter_map(|buf| { - if let RemoteBuffer::Loaded(buf) = buf { - Some(buf) - } else { - None - } - })) - } - }; - - let path = path.as_ref(); - open_buffers - .find(|buffer| { - if let Some(file) = buffer.upgrade(cx).and_then(|buffer| buffer.read(cx).file()) { - file.path().as_ref() == path - } else { - false - } - }) - .is_some() - } - - pub fn handle_update_buffer( - &mut self, - envelope: TypedEnvelope, - cx: &mut ModelContext, - ) -> Result<()> { - let payload = envelope.payload.clone(); - let buffer_id = payload.buffer_id as usize; - let ops = payload - .operations - .into_iter() - .map(|op| language::proto::deserialize_operation(op)) - .collect::, _>>()?; - - match self { - Worktree::Local(worktree) => { - let buffer = worktree - .open_buffers - .get(&buffer_id) - .and_then(|buf| buf.upgrade(cx)) - .ok_or_else(|| { - anyhow!("invalid buffer {} in update buffer message", buffer_id) - })?; - buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx))?; - } - Worktree::Remote(worktree) => match worktree.open_buffers.get_mut(&buffer_id) { - Some(RemoteBuffer::Operations(pending_ops)) => pending_ops.extend(ops), - Some(RemoteBuffer::Loaded(buffer)) => { - if let Some(buffer) = buffer.upgrade(cx) { - buffer.update(cx, |buffer, cx| buffer.apply_ops(ops, cx))?; - } else { - worktree - .open_buffers - .insert(buffer_id, RemoteBuffer::Operations(ops)); - } - } - None => { - worktree - .open_buffers - .insert(buffer_id, RemoteBuffer::Operations(ops)); - } - }, - } - - Ok(()) - } - - pub fn handle_save_buffer( - &mut self, - envelope: TypedEnvelope, - rpc: Arc, - cx: &mut ModelContext, - ) -> Result<()> { - let sender_id = envelope.original_sender_id()?; - let this = self.as_local().unwrap(); - let project_id = this - .share - .as_ref() - .ok_or_else(|| anyhow!("can't save buffer while disconnected"))? - .project_id; - - let buffer = this - .shared_buffers - .get(&sender_id) - .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned()) - .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?; - - let receipt = envelope.receipt(); - let worktree_id = envelope.payload.worktree_id; - let buffer_id = envelope.payload.buffer_id; - let save = cx.spawn(|_, mut cx| async move { - buffer.update(&mut cx, |buffer, cx| buffer.save(cx)).await - }); - - cx.background() - .spawn( - async move { - let (version, mtime) = save.await?; - - rpc.respond( - receipt, - proto::BufferSaved { - project_id, - worktree_id, - buffer_id, - version: (&version).into(), - mtime: Some(mtime.into()), - }, - ) - .await?; - - Ok(()) - } - .log_err(), - ) - .detach(); - - Ok(()) - } - - pub fn handle_buffer_saved( - &mut self, - envelope: TypedEnvelope, - cx: &mut ModelContext, - ) -> Result<()> { - let payload = envelope.payload.clone(); - let worktree = self.as_remote_mut().unwrap(); - if let Some(buffer) = worktree - .open_buffers - .get(&(payload.buffer_id as usize)) - .and_then(|buf| buf.upgrade(cx)) - { - buffer.update(cx, |buffer, cx| { - let version = payload.version.try_into()?; - let mtime = payload - .mtime - .ok_or_else(|| anyhow!("missing mtime"))? - .into(); - buffer.did_save(version, mtime, None, cx); - Result::<_, anyhow::Error>::Ok(()) - })?; - } - Ok(()) - } - - pub fn handle_format_buffer( - &mut self, - envelope: TypedEnvelope, - rpc: Arc, - cx: &mut ModelContext, - ) -> Result<()> { - let sender_id = envelope.original_sender_id()?; - let this = self.as_local().unwrap(); - let buffer = this - .shared_buffers - .get(&sender_id) - .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned()) - .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?; - - let receipt = envelope.receipt(); - cx.spawn(|_, mut cx| async move { - let format = buffer.update(&mut cx, |buffer, cx| buffer.format(cx)).await; - // We spawn here in order to enqueue the sending of `Ack` *after* transmission of edits - // associated with formatting. - cx.spawn(|_| async move { - match format { - Ok(()) => rpc.respond(receipt, proto::Ack {}).await?, - Err(error) => { - rpc.respond_with_error( - receipt, - proto::Error { - message: error.to_string(), - }, - ) - .await? - } - } - Ok::<_, anyhow::Error>(()) - }) - .await - .log_err(); - }) - .detach(); - - Ok(()) - } - fn poll_snapshot(&mut self, cx: &mut ModelContext) { match self { Self::Local(worktree) => { @@ -593,94 +319,18 @@ impl Worktree { } } else { worktree.poll_task.take(); - self.update_open_buffers(cx); + cx.emit(Event::UpdatedEntries); } } Self::Remote(worktree) => { worktree.snapshot = worktree.snapshot_rx.borrow().clone(); - self.update_open_buffers(cx); + cx.emit(Event::UpdatedEntries); } }; cx.notify(); } - fn update_open_buffers(&mut self, cx: &mut ModelContext) { - let open_buffers: Box> = match &self { - Self::Local(worktree) => Box::new(worktree.open_buffers.iter()), - Self::Remote(worktree) => { - Box::new(worktree.open_buffers.iter().filter_map(|(id, buf)| { - if let RemoteBuffer::Loaded(buf) = buf { - Some((id, buf)) - } else { - None - } - })) - } - }; - - let local = self.as_local().is_some(); - let worktree_path = self.abs_path.clone(); - let worktree_handle = cx.handle(); - let mut buffers_to_delete = Vec::new(); - for (buffer_id, buffer) in open_buffers { - if let Some(buffer) = buffer.upgrade(cx) { - buffer.update(cx, |buffer, cx| { - if let Some(old_file) = File::from_dyn(buffer.file()) { - let new_file = if let Some(entry) = old_file - .entry_id - .and_then(|entry_id| self.entry_for_id(entry_id)) - { - File { - is_local: local, - worktree_path: worktree_path.clone(), - entry_id: Some(entry.id), - mtime: entry.mtime, - path: entry.path.clone(), - worktree: worktree_handle.clone(), - } - } else if let Some(entry) = self.entry_for_path(old_file.path().as_ref()) { - File { - is_local: local, - worktree_path: worktree_path.clone(), - entry_id: Some(entry.id), - mtime: entry.mtime, - path: entry.path.clone(), - worktree: worktree_handle.clone(), - } - } else { - File { - is_local: local, - worktree_path: worktree_path.clone(), - entry_id: None, - path: old_file.path().clone(), - mtime: old_file.mtime(), - worktree: worktree_handle.clone(), - } - }; - - if let Some(task) = buffer.file_updated(Box::new(new_file), cx) { - task.detach(); - } - } - }); - } else { - buffers_to_delete.push(*buffer_id); - } - } - - for buffer_id in buffers_to_delete { - match self { - Self::Local(worktree) => { - worktree.open_buffers.remove(&buffer_id); - } - Self::Remote(worktree) => { - worktree.open_buffers.remove(&buffer_id); - } - } - } - } - fn send_buffer_update( &mut self, buffer_id: u64, @@ -770,9 +420,6 @@ pub struct LocalWorktree { poll_task: Option>, registration: Registration, share: Option, - loading_buffers: LoadingBuffers, - open_buffers: HashMap>, - shared_buffers: HashMap>>, diagnostics: HashMap, Vec>>, diagnostic_summaries: TreeMap, queued_operations: Vec<(u64, Operation)>, @@ -795,26 +442,17 @@ struct ShareState { } pub struct RemoteWorktree { + pub(crate) snapshot: Snapshot, project_id: u64, - snapshot: Snapshot, snapshot_rx: watch::Receiver, client: Arc, updates_tx: postage::mpsc::Sender, replica_id: ReplicaId, - loading_buffers: LoadingBuffers, - open_buffers: HashMap, queued_operations: Vec<(u64, Operation)>, diagnostic_summaries: TreeMap, weak: bool, } -type LoadingBuffers = HashMap< - Arc, - postage::watch::Receiver< - Option, Arc), Arc>>, - >, ->; - #[derive(Default, Deserialize)] struct WorktreeConfig { collaborators: Vec, @@ -883,9 +521,6 @@ impl LocalWorktree { registration: Registration::None, share: None, poll_task: None, - loading_buffers: Default::default(), - open_buffers: Default::default(), - shared_buffers: Default::default(), diagnostics: Default::default(), diagnostic_summaries: Default::default(), queued_operations: Default::default(), @@ -931,29 +566,7 @@ impl LocalWorktree { self.config.collaborators.clone() } - fn get_open_buffer( - &mut self, - path: &Path, - cx: &mut ModelContext, - ) -> Option> { - let handle = cx.handle(); - let mut result = None; - self.open_buffers.retain(|_buffer_id, buffer| { - if let Some(buffer) = buffer.upgrade(cx) { - if let Some(file) = File::from_dyn(buffer.read(cx).file()) { - if file.worktree == handle && file.path().as_ref() == path { - result = Some(buffer); - } - } - true - } else { - false - } - }); - result - } - - fn open_buffer( + pub(crate) fn open_buffer( &mut self, path: &Path, cx: &mut ModelContext, @@ -968,195 +581,22 @@ impl LocalWorktree { this.as_local_mut().unwrap().diagnostics.get(&path).cloned() }); - let mut buffer_operations = Vec::new(); - let buffer = cx.add_model(|cx| { + Ok(cx.add_model(|cx| { let mut buffer = Buffer::from_file(0, contents, Box::new(file), cx); if let Some(diagnostics) = diagnostics { - let op = buffer.update_diagnostics(None, diagnostics, cx).unwrap(); - buffer_operations.push(op); + buffer.update_diagnostics(None, diagnostics, cx).unwrap(); } buffer - }); - - this.update(&mut cx, |this, cx| { - for op in buffer_operations { - this.send_buffer_update(buffer.read(cx).remote_id(), op, cx); - } - let this = this.as_local_mut().unwrap(); - this.open_buffers.insert(buffer.id(), buffer.downgrade()); - }); - - Ok(buffer) + })) }) } - pub fn open_remote_buffer( - &mut self, - peer_id: PeerId, - buffer: ModelHandle, - cx: &mut ModelContext, - ) -> proto::OpenBufferResponse { - self.shared_buffers - .entry(peer_id) - .or_default() - .insert(buffer.id() as u64, buffer.clone()); - proto::OpenBufferResponse { - buffer: Some(buffer.update(cx.as_mut(), |buffer, _| buffer.to_proto())), - } - } - - pub fn close_remote_buffer( - &mut self, - envelope: TypedEnvelope, - cx: &mut ModelContext, - ) -> Result<()> { - if let Some(shared_buffers) = self.shared_buffers.get_mut(&envelope.original_sender_id()?) { - shared_buffers.remove(&envelope.payload.buffer_id); - cx.notify(); - } - - Ok(()) - } - - pub fn remove_collaborator( - &mut self, - peer_id: PeerId, - replica_id: ReplicaId, - cx: &mut ModelContext, - ) { - self.shared_buffers.remove(&peer_id); - for (_, buffer) in &self.open_buffers { - if let Some(buffer) = buffer.upgrade(cx) { - buffer.update(cx, |buffer, cx| buffer.remove_peer(replica_id, cx)); - } - } - cx.notify(); - } - pub fn update_diagnostics( &mut self, worktree_path: Arc, - params: lsp::PublishDiagnosticsParams, - disk_based_sources: &HashSet, - cx: &mut ModelContext, - ) -> Result<()> { - let mut next_group_id = 0; - let mut diagnostics = Vec::default(); - let mut primary_diagnostic_group_ids = HashMap::default(); - let mut sources_by_group_id = HashMap::default(); - let mut supporting_diagnostic_severities = HashMap::default(); - for diagnostic in ¶ms.diagnostics { - let source = diagnostic.source.as_ref(); - let code = diagnostic.code.as_ref().map(|code| match code { - lsp::NumberOrString::Number(code) => code.to_string(), - lsp::NumberOrString::String(code) => code.clone(), - }); - let range = range_from_lsp(diagnostic.range); - let is_supporting = diagnostic - .related_information - .as_ref() - .map_or(false, |infos| { - infos.iter().any(|info| { - primary_diagnostic_group_ids.contains_key(&( - source, - code.clone(), - range_from_lsp(info.location.range), - )) - }) - }); - - if is_supporting { - if let Some(severity) = diagnostic.severity { - supporting_diagnostic_severities - .insert((source, code.clone(), range), severity); - } - } else { - let group_id = post_inc(&mut next_group_id); - let is_disk_based = - source.map_or(false, |source| disk_based_sources.contains(source)); - - sources_by_group_id.insert(group_id, source); - primary_diagnostic_group_ids - .insert((source, code.clone(), range.clone()), group_id); - - diagnostics.push(DiagnosticEntry { - range, - diagnostic: Diagnostic { - code: code.clone(), - severity: diagnostic.severity.unwrap_or(DiagnosticSeverity::ERROR), - message: diagnostic.message.clone(), - group_id, - is_primary: true, - is_valid: true, - is_disk_based, - }, - }); - if let Some(infos) = &diagnostic.related_information { - for info in infos { - if info.location.uri == params.uri { - let range = range_from_lsp(info.location.range); - diagnostics.push(DiagnosticEntry { - range, - diagnostic: Diagnostic { - code: code.clone(), - severity: DiagnosticSeverity::INFORMATION, - message: info.message.clone(), - group_id, - is_primary: false, - is_valid: true, - is_disk_based, - }, - }); - } - } - } - } - } - - for entry in &mut diagnostics { - let diagnostic = &mut entry.diagnostic; - if !diagnostic.is_primary { - let source = *sources_by_group_id.get(&diagnostic.group_id).unwrap(); - if let Some(&severity) = supporting_diagnostic_severities.get(&( - source, - diagnostic.code.clone(), - entry.range.clone(), - )) { - diagnostic.severity = severity; - } - } - } - - self.update_diagnostic_entries(worktree_path, params.version, diagnostics, cx)?; - Ok(()) - } - - pub fn update_diagnostic_entries( - &mut self, - worktree_path: Arc, - version: Option, diagnostics: Vec>, cx: &mut ModelContext, ) -> Result<()> { - for buffer in self.open_buffers.values() { - if let Some(buffer) = buffer.upgrade(cx) { - if buffer - .read(cx) - .file() - .map_or(false, |file| *file.path() == worktree_path) - { - let (remote_id, operation) = buffer.update(cx, |buffer, cx| { - ( - buffer.remote_id(), - buffer.update_diagnostics(version, diagnostics.clone(), cx), - ) - }); - self.send_buffer_update(remote_id, operation?, cx); - break; - } - } - } - let summary = DiagnosticSummary::new(&diagnostics); self.diagnostic_summaries .insert(PathKey(worktree_path.clone()), summary.clone()); @@ -1192,40 +632,6 @@ impl LocalWorktree { Ok(()) } - fn send_buffer_update( - &mut self, - buffer_id: u64, - operation: Operation, - cx: &mut ModelContext, - ) -> Option<()> { - let share = self.share.as_ref()?; - let project_id = share.project_id; - let worktree_id = self.id(); - let rpc = self.client.clone(); - cx.spawn(|worktree, mut cx| async move { - if let Err(error) = rpc - .request(proto::UpdateBuffer { - project_id, - worktree_id: worktree_id.0 as u64, - buffer_id, - operations: vec![language::proto::serialize_operation(&operation)], - }) - .await - { - worktree.update(&mut cx, |worktree, _| { - log::error!("error sending buffer operation: {}", error); - worktree - .as_local_mut() - .unwrap() - .queued_operations - .push((buffer_id, operation)); - }); - } - }) - .detach(); - None - } - pub fn scan_complete(&self) -> impl Future { let mut scan_state_rx = self.last_scan_state_rx.clone(); async move { @@ -1248,22 +654,6 @@ impl LocalWorktree { self.snapshot.clone() } - pub fn abs_path(&self) -> &Arc { - &self.snapshot.abs_path - } - - pub fn contains_abs_path(&self, path: &Path) -> bool { - path.starts_with(&self.snapshot.abs_path) - } - - fn absolutize(&self, path: &Path) -> PathBuf { - if path.file_name().is_some() { - self.snapshot.abs_path.join(path) - } else { - self.snapshot.abs_path.to_path_buf() - } - } - fn load(&self, path: &Path, cx: &mut ModelContext) -> Task> { let handle = cx.handle(); let path = Arc::from(path); @@ -1304,8 +694,6 @@ impl LocalWorktree { let entry = save.await?; let file = this.update(&mut cx, |this, cx| { let this = this.as_local_mut().unwrap(); - this.open_buffers - .insert(buffer_handle.id(), buffer_handle.downgrade()); File { entry_id: Some(entry.id), worktree: cx.handle(), @@ -1485,29 +873,7 @@ impl fmt::Debug for LocalWorktree { } impl RemoteWorktree { - fn get_open_buffer( - &mut self, - path: &Path, - cx: &mut ModelContext, - ) -> Option> { - let handle = cx.handle(); - let mut existing_buffer = None; - self.open_buffers.retain(|_buffer_id, buffer| { - if let Some(buffer) = buffer.upgrade(cx.as_ref()) { - if let Some(file) = File::from_dyn(buffer.read(cx).file()) { - if file.worktree == handle && file.path().as_ref() == path { - existing_buffer = Some(buffer); - } - } - true - } else { - false - } - }); - existing_buffer - } - - fn open_buffer( + pub(crate) fn open_buffer( &mut self, path: &Path, cx: &mut ModelContext, @@ -1545,20 +911,9 @@ impl RemoteWorktree { is_local: false, }; let remote_buffer = response.buffer.ok_or_else(|| anyhow!("empty buffer"))?; - let buffer_id = remote_buffer.id as usize; - let buffer = cx.add_model(|cx| { + Ok(cx.add_model(|cx| { Buffer::from_proto(replica_id, remote_buffer, Some(Box::new(file)), cx).unwrap() - }); - this.update(&mut cx, move |this, cx| { - let this = this.as_remote_mut().unwrap(); - if let Some(RemoteBuffer::Operations(pending_ops)) = this - .open_buffers - .insert(buffer_id, RemoteBuffer::Loaded(buffer.downgrade())) - { - buffer.update(cx, |buf, cx| buf.apply_ops(pending_ops, cx))?; - } - Result::<_, anyhow::Error>::Ok(buffer) - }) + })) }) } @@ -1597,29 +952,6 @@ impl RemoteWorktree { }, ); } - - pub fn remove_collaborator(&mut self, replica_id: ReplicaId, cx: &mut ModelContext) { - for (_, buffer) in &self.open_buffers { - if let Some(buffer) = buffer.upgrade(cx) { - buffer.update(cx, |buffer, cx| buffer.remove_peer(replica_id, cx)); - } - } - cx.notify(); - } -} - -enum RemoteBuffer { - Operations(Vec), - Loaded(WeakModelHandle), -} - -impl RemoteBuffer { - fn upgrade(&self, cx: &impl UpgradeModelHandle) -> Option> { - match self { - Self::Operations(_) => None, - Self::Loaded(buffer) => buffer.upgrade(cx), - } - } } impl Snapshot { @@ -1627,7 +959,7 @@ impl Snapshot { self.id } - pub fn to_proto( + pub(crate) fn to_proto( &self, diagnostic_summaries: &TreeMap, weak: bool, @@ -1650,7 +982,7 @@ impl Snapshot { } } - pub fn build_update( + pub(crate) fn build_update( &self, other: &Self, project_id: u64, @@ -1715,7 +1047,7 @@ impl Snapshot { } } - fn apply_update(&mut self, update: proto::UpdateWorktree) -> Result<()> { + pub(crate) fn apply_update(&mut self, update: proto::UpdateWorktree) -> Result<()> { self.scan_id += 1; let scan_id = self.scan_id; @@ -1826,6 +1158,22 @@ impl Snapshot { } } + pub fn contains_abs_path(&self, path: &Path) -> bool { + path.starts_with(&self.abs_path) + } + + fn absolutize(&self, path: &Path) -> PathBuf { + if path.file_name().is_some() { + self.abs_path.join(path) + } else { + self.abs_path.to_path_buf() + } + } + + pub fn abs_path(&self) -> &Arc { + &self.abs_path + } + pub fn root_entry(&self) -> Option<&Entry> { self.entry_for_path("") } @@ -2006,12 +1354,12 @@ impl fmt::Debug for Snapshot { #[derive(Clone, PartialEq)] pub struct File { - entry_id: Option, pub worktree: ModelHandle, - worktree_path: Arc, pub path: Arc, pub mtime: SystemTime, - is_local: bool, + pub(crate) entry_id: Option, + pub(crate) worktree_path: Arc, + pub(crate) is_local: bool, } impl language::File for File { @@ -3062,18 +2410,13 @@ mod tests { use anyhow::Result; use client::test::FakeHttpClient; use fs::RealFs; - use language::{Diagnostic, DiagnosticEntry}; - use lsp::Url; use rand::prelude::*; use serde_json::json; - use std::{cell::RefCell, rc::Rc}; use std::{ env, fmt::Write, time::{SystemTime, UNIX_EPOCH}, }; - use text::Point; - use unindent::Unindent as _; use util::test::temp_tree; #[gpui::test] @@ -3094,7 +2437,7 @@ mod tests { let http_client = FakeHttpClient::with_404_response(); let client = Client::new(http_client); - let tree = Worktree::open_local( + let tree = Worktree::local( client, Arc::from(Path::new("/root")), false, @@ -3121,225 +2464,6 @@ mod tests { }) } - #[gpui::test] - async fn test_save_file(mut cx: gpui::TestAppContext) { - let dir = temp_tree(json!({ - "file1": "the old contents", - })); - - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client); - - let tree = Worktree::open_local( - client, - dir.path(), - false, - Arc::new(RealFs), - &mut cx.to_async(), - ) - .await - .unwrap(); - let (buffer, _) = tree - .update(&mut cx, |tree, cx| tree.open_buffer("file1", cx)) - .await - .unwrap(); - let save = buffer.update(&mut cx, |buffer, cx| { - buffer.edit(Some(0..0), "a line of text.\n".repeat(10 * 1024), cx); - buffer.save(cx) - }); - save.await.unwrap(); - - let new_text = std::fs::read_to_string(dir.path().join("file1")).unwrap(); - assert_eq!(new_text, buffer.read_with(&cx, |buffer, _| buffer.text())); - } - - #[gpui::test] - async fn test_save_in_single_file_worktree(mut cx: gpui::TestAppContext) { - let dir = temp_tree(json!({ - "file1": "the old contents", - })); - let file_path = dir.path().join("file1"); - - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client); - - let tree = Worktree::open_local( - client, - file_path.clone(), - false, - Arc::new(RealFs), - &mut cx.to_async(), - ) - .await - .unwrap(); - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; - cx.read(|cx| assert_eq!(tree.read(cx).file_count(), 1)); - - let (buffer, _) = tree - .update(&mut cx, |tree, cx| tree.open_buffer("", cx)) - .await - .unwrap(); - let save = buffer.update(&mut cx, |buffer, cx| { - buffer.edit(Some(0..0), "a line of text.\n".repeat(10 * 1024), cx); - buffer.save(cx) - }); - save.await.unwrap(); - - let new_text = std::fs::read_to_string(file_path).unwrap(); - assert_eq!(new_text, buffer.read_with(&cx, |buffer, _| buffer.text())); - } - - #[gpui::test] - async fn test_rescan_and_remote_updates(mut cx: gpui::TestAppContext) { - let dir = temp_tree(json!({ - "a": { - "file1": "", - "file2": "", - "file3": "", - }, - "b": { - "c": { - "file4": "", - "file5": "", - } - } - })); - - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - let tree = Worktree::open_local( - client, - dir.path(), - false, - Arc::new(RealFs), - &mut cx.to_async(), - ) - .await - .unwrap(); - - let buffer_for_path = |path: &'static str, cx: &mut gpui::TestAppContext| { - let buffer = tree.update(cx, |tree, cx| tree.open_buffer(path, cx)); - async move { buffer.await.unwrap().0 } - }; - let id_for_path = |path: &'static str, cx: &gpui::TestAppContext| { - tree.read_with(cx, |tree, _| { - tree.entry_for_path(path) - .expect(&format!("no entry for path {}", path)) - .id - }) - }; - - let buffer2 = buffer_for_path("a/file2", &mut cx).await; - let buffer3 = buffer_for_path("a/file3", &mut cx).await; - let buffer4 = buffer_for_path("b/c/file4", &mut cx).await; - let buffer5 = buffer_for_path("b/c/file5", &mut cx).await; - - let file2_id = id_for_path("a/file2", &cx); - let file3_id = id_for_path("a/file3", &cx); - let file4_id = id_for_path("b/c/file4", &cx); - - // Wait for the initial scan. - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; - - // Create a remote copy of this worktree. - let initial_snapshot = tree.read_with(&cx, |tree, _| tree.snapshot()); - let remote = Worktree::remote( - 1, - 1, - initial_snapshot.to_proto(&Default::default(), Default::default()), - Client::new(http_client.clone()), - &mut cx.to_async(), - ) - .await - .unwrap(); - - cx.read(|cx| { - assert!(!buffer2.read(cx).is_dirty()); - assert!(!buffer3.read(cx).is_dirty()); - assert!(!buffer4.read(cx).is_dirty()); - assert!(!buffer5.read(cx).is_dirty()); - }); - - // Rename and delete files and directories. - tree.flush_fs_events(&cx).await; - std::fs::rename(dir.path().join("a/file3"), dir.path().join("b/c/file3")).unwrap(); - std::fs::remove_file(dir.path().join("b/c/file5")).unwrap(); - std::fs::rename(dir.path().join("b/c"), dir.path().join("d")).unwrap(); - std::fs::rename(dir.path().join("a/file2"), dir.path().join("a/file2.new")).unwrap(); - tree.flush_fs_events(&cx).await; - - let expected_paths = vec![ - "a", - "a/file1", - "a/file2.new", - "b", - "d", - "d/file3", - "d/file4", - ]; - - cx.read(|app| { - assert_eq!( - tree.read(app) - .paths() - .map(|p| p.to_str().unwrap()) - .collect::>(), - expected_paths - ); - - assert_eq!(id_for_path("a/file2.new", &cx), file2_id); - assert_eq!(id_for_path("d/file3", &cx), file3_id); - assert_eq!(id_for_path("d/file4", &cx), file4_id); - - assert_eq!( - buffer2.read(app).file().unwrap().path().as_ref(), - Path::new("a/file2.new") - ); - assert_eq!( - buffer3.read(app).file().unwrap().path().as_ref(), - Path::new("d/file3") - ); - assert_eq!( - buffer4.read(app).file().unwrap().path().as_ref(), - Path::new("d/file4") - ); - assert_eq!( - buffer5.read(app).file().unwrap().path().as_ref(), - Path::new("b/c/file5") - ); - - assert!(!buffer2.read(app).file().unwrap().is_deleted()); - assert!(!buffer3.read(app).file().unwrap().is_deleted()); - assert!(!buffer4.read(app).file().unwrap().is_deleted()); - assert!(buffer5.read(app).file().unwrap().is_deleted()); - }); - - // Update the remote worktree. Check that it becomes consistent with the - // local worktree. - remote.update(&mut cx, |remote, cx| { - let update_message = - tree.read(cx) - .snapshot() - .build_update(&initial_snapshot, 1, 1, true); - remote - .as_remote_mut() - .unwrap() - .snapshot - .apply_update(update_message) - .unwrap(); - - assert_eq!( - remote - .paths() - .map(|p| p.to_str().unwrap()) - .collect::>(), - expected_paths - ); - }); - } - #[gpui::test] async fn test_rescan_with_gitignore(cx: gpui::TestAppContext) { let dir = temp_tree(json!({ @@ -3356,7 +2480,7 @@ mod tests { let http_client = FakeHttpClient::with_404_response(); let client = Client::new(http_client.clone()); - let tree = Worktree::open_local( + let tree = Worktree::local( client, dir.path(), false, @@ -3390,564 +2514,6 @@ mod tests { }); } - #[gpui::test] - async fn test_buffer_deduping(mut cx: gpui::TestAppContext) { - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client); - - let fs = Arc::new(FakeFs::new()); - fs.insert_tree( - "/the-dir", - json!({ - "a.txt": "a-contents", - "b.txt": "b-contents", - }), - ) - .await; - - let worktree = - Worktree::open_local(client, "/the-dir".as_ref(), false, fs, &mut cx.to_async()) - .await - .unwrap(); - - // Spawn multiple tasks to open paths, repeating some paths. - let (buffer_a_1, buffer_b, buffer_a_2) = worktree.update(&mut cx, |worktree, cx| { - ( - worktree.open_buffer("a.txt", cx), - worktree.open_buffer("b.txt", cx), - worktree.open_buffer("a.txt", cx), - ) - }); - - let buffer_a_1 = buffer_a_1.await.unwrap().0; - let buffer_a_2 = buffer_a_2.await.unwrap().0; - let buffer_b = buffer_b.await.unwrap().0; - assert_eq!(buffer_a_1.read_with(&cx, |b, _| b.text()), "a-contents"); - assert_eq!(buffer_b.read_with(&cx, |b, _| b.text()), "b-contents"); - - // There is only one buffer per path. - let buffer_a_id = buffer_a_1.id(); - assert_eq!(buffer_a_2.id(), buffer_a_id); - - // Open the same path again while it is still open. - drop(buffer_a_1); - let buffer_a_3 = worktree - .update(&mut cx, |worktree, cx| worktree.open_buffer("a.txt", cx)) - .await - .unwrap() - .0; - - // There's still only one buffer per path. - assert_eq!(buffer_a_3.id(), buffer_a_id); - } - - #[gpui::test] - async fn test_buffer_is_dirty(mut cx: gpui::TestAppContext) { - use std::fs; - - let dir = temp_tree(json!({ - "file1": "abc", - "file2": "def", - "file3": "ghi", - })); - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - - let tree = Worktree::open_local( - client, - dir.path(), - false, - Arc::new(RealFs), - &mut cx.to_async(), - ) - .await - .unwrap(); - tree.flush_fs_events(&cx).await; - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; - - let (buffer1, _) = tree - .update(&mut cx, |tree, cx| tree.open_buffer("file1", cx)) - .await - .unwrap(); - let events = Rc::new(RefCell::new(Vec::new())); - - // initially, the buffer isn't dirty. - buffer1.update(&mut cx, |buffer, cx| { - cx.subscribe(&buffer1, { - let events = events.clone(); - move |_, _, event, _| events.borrow_mut().push(event.clone()) - }) - .detach(); - - assert!(!buffer.is_dirty()); - assert!(events.borrow().is_empty()); - - buffer.edit(vec![1..2], "", cx); - }); - - // after the first edit, the buffer is dirty, and emits a dirtied event. - buffer1.update(&mut cx, |buffer, cx| { - assert!(buffer.text() == "ac"); - assert!(buffer.is_dirty()); - assert_eq!( - *events.borrow(), - &[language::Event::Edited, language::Event::Dirtied] - ); - events.borrow_mut().clear(); - buffer.did_save(buffer.version(), buffer.file().unwrap().mtime(), None, cx); - }); - - // after saving, the buffer is not dirty, and emits a saved event. - buffer1.update(&mut cx, |buffer, cx| { - assert!(!buffer.is_dirty()); - assert_eq!(*events.borrow(), &[language::Event::Saved]); - events.borrow_mut().clear(); - - buffer.edit(vec![1..1], "B", cx); - buffer.edit(vec![2..2], "D", cx); - }); - - // after editing again, the buffer is dirty, and emits another dirty event. - buffer1.update(&mut cx, |buffer, cx| { - assert!(buffer.text() == "aBDc"); - assert!(buffer.is_dirty()); - assert_eq!( - *events.borrow(), - &[ - language::Event::Edited, - language::Event::Dirtied, - language::Event::Edited, - ], - ); - events.borrow_mut().clear(); - - // TODO - currently, after restoring the buffer to its - // previously-saved state, the is still considered dirty. - buffer.edit([1..3], "", cx); - assert!(buffer.text() == "ac"); - assert!(buffer.is_dirty()); - }); - - assert_eq!(*events.borrow(), &[language::Event::Edited]); - - // When a file is deleted, the buffer is considered dirty. - let events = Rc::new(RefCell::new(Vec::new())); - let (buffer2, _) = tree - .update(&mut cx, |tree, cx| tree.open_buffer("file2", cx)) - .await - .unwrap(); - buffer2.update(&mut cx, |_, cx| { - cx.subscribe(&buffer2, { - let events = events.clone(); - move |_, _, event, _| events.borrow_mut().push(event.clone()) - }) - .detach(); - }); - - fs::remove_file(dir.path().join("file2")).unwrap(); - buffer2.condition(&cx, |b, _| b.is_dirty()).await; - assert_eq!( - *events.borrow(), - &[language::Event::Dirtied, language::Event::FileHandleChanged] - ); - - // When a file is already dirty when deleted, we don't emit a Dirtied event. - let events = Rc::new(RefCell::new(Vec::new())); - let (buffer3, _) = tree - .update(&mut cx, |tree, cx| tree.open_buffer("file3", cx)) - .await - .unwrap(); - buffer3.update(&mut cx, |_, cx| { - cx.subscribe(&buffer3, { - let events = events.clone(); - move |_, _, event, _| events.borrow_mut().push(event.clone()) - }) - .detach(); - }); - - tree.flush_fs_events(&cx).await; - buffer3.update(&mut cx, |buffer, cx| { - buffer.edit(Some(0..0), "x", cx); - }); - events.borrow_mut().clear(); - fs::remove_file(dir.path().join("file3")).unwrap(); - buffer3 - .condition(&cx, |_, _| !events.borrow().is_empty()) - .await; - assert_eq!(*events.borrow(), &[language::Event::FileHandleChanged]); - cx.read(|cx| assert!(buffer3.read(cx).is_dirty())); - } - - #[gpui::test] - async fn test_buffer_file_changes_on_disk(mut cx: gpui::TestAppContext) { - use std::fs; - - let initial_contents = "aaa\nbbbbb\nc\n"; - let dir = temp_tree(json!({ "the-file": initial_contents })); - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client); - - let tree = Worktree::open_local( - client, - dir.path(), - false, - Arc::new(RealFs), - &mut cx.to_async(), - ) - .await - .unwrap(); - cx.read(|cx| tree.read(cx).as_local().unwrap().scan_complete()) - .await; - - let abs_path = dir.path().join("the-file"); - let (buffer, _) = tree - .update(&mut cx, |tree, cx| { - tree.open_buffer(Path::new("the-file"), cx) - }) - .await - .unwrap(); - - // TODO - // Add a cursor on each row. - // let selection_set_id = buffer.update(&mut cx, |buffer, cx| { - // assert!(!buffer.is_dirty()); - // buffer.add_selection_set( - // &(0..3) - // .map(|row| Selection { - // id: row as usize, - // start: Point::new(row, 1), - // end: Point::new(row, 1), - // reversed: false, - // goal: SelectionGoal::None, - // }) - // .collect::>(), - // cx, - // ) - // }); - - // Change the file on disk, adding two new lines of text, and removing - // one line. - buffer.read_with(&cx, |buffer, _| { - assert!(!buffer.is_dirty()); - assert!(!buffer.has_conflict()); - }); - let new_contents = "AAAA\naaa\nBB\nbbbbb\n"; - fs::write(&abs_path, new_contents).unwrap(); - - // Because the buffer was not modified, it is reloaded from disk. Its - // contents are edited according to the diff between the old and new - // file contents. - buffer - .condition(&cx, |buffer, _| buffer.text() == new_contents) - .await; - - buffer.update(&mut cx, |buffer, _| { - assert_eq!(buffer.text(), new_contents); - assert!(!buffer.is_dirty()); - assert!(!buffer.has_conflict()); - - // TODO - // let cursor_positions = buffer - // .selection_set(selection_set_id) - // .unwrap() - // .selections::(&*buffer) - // .map(|selection| { - // assert_eq!(selection.start, selection.end); - // selection.start - // }) - // .collect::>(); - // assert_eq!( - // cursor_positions, - // [Point::new(1, 1), Point::new(3, 1), Point::new(4, 0)] - // ); - }); - - // Modify the buffer - buffer.update(&mut cx, |buffer, cx| { - buffer.edit(vec![0..0], " ", cx); - assert!(buffer.is_dirty()); - assert!(!buffer.has_conflict()); - }); - - // Change the file on disk again, adding blank lines to the beginning. - fs::write(&abs_path, "\n\n\nAAAA\naaa\nBB\nbbbbb\n").unwrap(); - - // Because the buffer is modified, it doesn't reload from disk, but is - // marked as having a conflict. - buffer - .condition(&cx, |buffer, _| buffer.has_conflict()) - .await; - } - - #[gpui::test] - async fn test_grouped_diagnostics(mut cx: gpui::TestAppContext) { - let fs = Arc::new(FakeFs::new()); - let http_client = FakeHttpClient::with_404_response(); - let client = Client::new(http_client.clone()); - - fs.insert_tree( - "/the-dir", - json!({ - "a.rs": " - fn foo(mut v: Vec) { - for x in &v { - v.push(1); - } - } - " - .unindent(), - }), - ) - .await; - - let worktree = Worktree::open_local( - client.clone(), - "/the-dir".as_ref(), - false, - fs, - &mut cx.to_async(), - ) - .await - .unwrap(); - - let (buffer, _) = worktree - .update(&mut cx, |tree, cx| tree.open_buffer("a.rs", cx)) - .await - .unwrap(); - - let buffer_uri = Url::from_file_path("/the-dir/a.rs").unwrap(); - let message = lsp::PublishDiagnosticsParams { - uri: buffer_uri.clone(), - diagnostics: vec![ - lsp::Diagnostic { - range: lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9)), - severity: Some(DiagnosticSeverity::WARNING), - message: "error 1".to_string(), - related_information: Some(vec![lsp::DiagnosticRelatedInformation { - location: lsp::Location { - uri: buffer_uri.clone(), - range: lsp::Range::new( - lsp::Position::new(1, 8), - lsp::Position::new(1, 9), - ), - }, - message: "error 1 hint 1".to_string(), - }]), - ..Default::default() - }, - lsp::Diagnostic { - range: lsp::Range::new(lsp::Position::new(1, 8), lsp::Position::new(1, 9)), - severity: Some(DiagnosticSeverity::HINT), - message: "error 1 hint 1".to_string(), - related_information: Some(vec![lsp::DiagnosticRelatedInformation { - location: lsp::Location { - uri: buffer_uri.clone(), - range: lsp::Range::new( - lsp::Position::new(1, 8), - lsp::Position::new(1, 9), - ), - }, - message: "original diagnostic".to_string(), - }]), - ..Default::default() - }, - lsp::Diagnostic { - range: lsp::Range::new(lsp::Position::new(2, 8), lsp::Position::new(2, 17)), - severity: Some(DiagnosticSeverity::ERROR), - message: "error 2".to_string(), - related_information: Some(vec![ - lsp::DiagnosticRelatedInformation { - location: lsp::Location { - uri: buffer_uri.clone(), - range: lsp::Range::new( - lsp::Position::new(1, 13), - lsp::Position::new(1, 15), - ), - }, - message: "error 2 hint 1".to_string(), - }, - lsp::DiagnosticRelatedInformation { - location: lsp::Location { - uri: buffer_uri.clone(), - range: lsp::Range::new( - lsp::Position::new(1, 13), - lsp::Position::new(1, 15), - ), - }, - message: "error 2 hint 2".to_string(), - }, - ]), - ..Default::default() - }, - lsp::Diagnostic { - range: lsp::Range::new(lsp::Position::new(1, 13), lsp::Position::new(1, 15)), - severity: Some(DiagnosticSeverity::HINT), - message: "error 2 hint 1".to_string(), - related_information: Some(vec![lsp::DiagnosticRelatedInformation { - location: lsp::Location { - uri: buffer_uri.clone(), - range: lsp::Range::new( - lsp::Position::new(2, 8), - lsp::Position::new(2, 17), - ), - }, - message: "original diagnostic".to_string(), - }]), - ..Default::default() - }, - lsp::Diagnostic { - range: lsp::Range::new(lsp::Position::new(1, 13), lsp::Position::new(1, 15)), - severity: Some(DiagnosticSeverity::HINT), - message: "error 2 hint 2".to_string(), - related_information: Some(vec![lsp::DiagnosticRelatedInformation { - location: lsp::Location { - uri: buffer_uri.clone(), - range: lsp::Range::new( - lsp::Position::new(2, 8), - lsp::Position::new(2, 17), - ), - }, - message: "original diagnostic".to_string(), - }]), - ..Default::default() - }, - ], - version: None, - }; - - worktree - .update(&mut cx, |tree, cx| { - tree.as_local_mut().unwrap().update_diagnostics( - Arc::from("a.rs".as_ref()), - message, - &Default::default(), - cx, - ) - }) - .unwrap(); - let buffer = buffer.read_with(&cx, |buffer, _| buffer.snapshot()); - - assert_eq!( - buffer - .diagnostics_in_range::<_, Point>(0..buffer.len()) - .collect::>(), - &[ - DiagnosticEntry { - range: Point::new(1, 8)..Point::new(1, 9), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::WARNING, - message: "error 1".to_string(), - group_id: 0, - is_primary: true, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(1, 8)..Point::new(1, 9), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::HINT, - message: "error 1 hint 1".to_string(), - group_id: 0, - is_primary: false, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(1, 13)..Point::new(1, 15), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::HINT, - message: "error 2 hint 1".to_string(), - group_id: 1, - is_primary: false, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(1, 13)..Point::new(1, 15), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::HINT, - message: "error 2 hint 2".to_string(), - group_id: 1, - is_primary: false, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(2, 8)..Point::new(2, 17), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::ERROR, - message: "error 2".to_string(), - group_id: 1, - is_primary: true, - ..Default::default() - } - } - ] - ); - - assert_eq!( - buffer.diagnostic_group::(0).collect::>(), - &[ - DiagnosticEntry { - range: Point::new(1, 8)..Point::new(1, 9), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::WARNING, - message: "error 1".to_string(), - group_id: 0, - is_primary: true, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(1, 8)..Point::new(1, 9), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::HINT, - message: "error 1 hint 1".to_string(), - group_id: 0, - is_primary: false, - ..Default::default() - } - }, - ] - ); - assert_eq!( - buffer.diagnostic_group::(1).collect::>(), - &[ - DiagnosticEntry { - range: Point::new(1, 13)..Point::new(1, 15), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::HINT, - message: "error 2 hint 1".to_string(), - group_id: 1, - is_primary: false, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(1, 13)..Point::new(1, 15), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::HINT, - message: "error 2 hint 2".to_string(), - group_id: 1, - is_primary: false, - ..Default::default() - } - }, - DiagnosticEntry { - range: Point::new(2, 8)..Point::new(2, 17), - diagnostic: Diagnostic { - severity: DiagnosticSeverity::ERROR, - message: "error 2".to_string(), - group_id: 1, - is_primary: true, - ..Default::default() - } - } - ] - ); - } - #[gpui::test(iterations = 100)] fn test_random(mut rng: StdRng) { let operations = env::var("OPERATIONS") From 03dc1e5aea9f00e33b32dc46cc3531a63d8d01f3 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 16:10:26 -0800 Subject: [PATCH 04/12] Move main worktree structs adjacent to each other --- crates/project/src/worktree.rs | 266 ++++++++++++++++----------------- 1 file changed, 133 insertions(+), 133 deletions(-) diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 5bdc2221c16f7bd477664fb699e6ca86983f4de9..487411f97acd13b75e3e0508a3ccd38cd4da1915 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -46,6 +46,57 @@ lazy_static! { static ref GITIGNORE: &'static OsStr = OsStr::new(".gitignore"); } +#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)] +pub struct WorktreeId(usize); + +pub enum Worktree { + Local(LocalWorktree), + Remote(RemoteWorktree), +} + +pub struct LocalWorktree { + snapshot: Snapshot, + config: WorktreeConfig, + background_snapshot: Arc>, + last_scan_state_rx: watch::Receiver, + _background_scanner_task: Option>, + poll_task: Option>, + registration: Registration, + share: Option, + diagnostics: HashMap, Vec>>, + diagnostic_summaries: TreeMap, + queued_operations: Vec<(u64, Operation)>, + client: Arc, + fs: Arc, + weak: bool, +} + +pub struct RemoteWorktree { + pub(crate) snapshot: Snapshot, + project_id: u64, + snapshot_rx: watch::Receiver, + client: Arc, + updates_tx: postage::mpsc::Sender, + replica_id: ReplicaId, + queued_operations: Vec<(u64, Operation)>, + diagnostic_summaries: TreeMap, + weak: bool, +} + +#[derive(Clone)] +pub struct Snapshot { + id: WorktreeId, + scan_id: usize, + abs_path: Arc, + root_name: String, + root_char_bag: CharBag, + ignores: HashMap, (Arc, usize)>, + entries_by_path: SumTree, + entries_by_id: SumTree, + removed_entry_ids: HashMap, + next_entry_id: Arc, +} + #[derive(Clone, Debug)] enum ScanState { Idle, @@ -53,12 +104,22 @@ enum ScanState { Err(Arc), } -#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash, PartialOrd, Ord)] -pub struct WorktreeId(usize); +#[derive(Debug, Eq, PartialEq)] +enum Registration { + None, + Pending, + Done { project_id: u64 }, +} -pub enum Worktree { - Local(LocalWorktree), - Remote(RemoteWorktree), +struct ShareState { + project_id: u64, + snapshots_tx: Sender, + _maintain_remote_snapshot: Option>, +} + +#[derive(Default, Deserialize)] +struct WorktreeConfig { + collaborators: Vec, } pub enum Event { @@ -373,91 +434,6 @@ impl Worktree { } } -impl WorktreeId { - pub fn from_usize(handle_id: usize) -> Self { - Self(handle_id) - } - - pub(crate) fn from_proto(id: u64) -> Self { - Self(id as usize) - } - - pub fn to_proto(&self) -> u64 { - self.0 as u64 - } - - pub fn to_usize(&self) -> usize { - self.0 - } -} - -impl fmt::Display for WorktreeId { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.0.fmt(f) - } -} - -#[derive(Clone)] -pub struct Snapshot { - id: WorktreeId, - scan_id: usize, - abs_path: Arc, - root_name: String, - root_char_bag: CharBag, - ignores: HashMap, (Arc, usize)>, - entries_by_path: SumTree, - entries_by_id: SumTree, - removed_entry_ids: HashMap, - next_entry_id: Arc, -} - -pub struct LocalWorktree { - snapshot: Snapshot, - config: WorktreeConfig, - background_snapshot: Arc>, - last_scan_state_rx: watch::Receiver, - _background_scanner_task: Option>, - poll_task: Option>, - registration: Registration, - share: Option, - diagnostics: HashMap, Vec>>, - diagnostic_summaries: TreeMap, - queued_operations: Vec<(u64, Operation)>, - client: Arc, - fs: Arc, - weak: bool, -} - -#[derive(Debug, Eq, PartialEq)] -enum Registration { - None, - Pending, - Done { project_id: u64 }, -} - -struct ShareState { - project_id: u64, - snapshots_tx: Sender, - _maintain_remote_snapshot: Option>, -} - -pub struct RemoteWorktree { - pub(crate) snapshot: Snapshot, - project_id: u64, - snapshot_rx: watch::Receiver, - client: Arc, - updates_tx: postage::mpsc::Sender, - replica_id: ReplicaId, - queued_operations: Vec<(u64, Operation)>, - diagnostic_summaries: TreeMap, - weak: bool, -} - -#[derive(Default, Deserialize)] -struct WorktreeConfig { - collaborators: Vec, -} - impl LocalWorktree { async fn new( client: Arc, @@ -829,49 +805,6 @@ impl LocalWorktree { } } -fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result { - let contents = smol::block_on(fs.load(&abs_path))?; - let parent = abs_path.parent().unwrap_or(Path::new("/")); - let mut builder = GitignoreBuilder::new(parent); - for line in contents.lines() { - builder.add_line(Some(abs_path.into()), line)?; - } - Ok(builder.build()?) -} - -impl Deref for Worktree { - type Target = Snapshot; - - fn deref(&self) -> &Self::Target { - match self { - Worktree::Local(worktree) => &worktree.snapshot, - Worktree::Remote(worktree) => &worktree.snapshot, - } - } -} - -impl Deref for LocalWorktree { - type Target = Snapshot; - - fn deref(&self) -> &Self::Target { - &self.snapshot - } -} - -impl Deref for RemoteWorktree { - type Target = Snapshot; - - fn deref(&self) -> &Self::Target { - &self.snapshot - } -} - -impl fmt::Debug for LocalWorktree { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - self.snapshot.fmt(f) - } -} - impl RemoteWorktree { pub(crate) fn open_buffer( &mut self, @@ -1340,6 +1273,73 @@ impl Snapshot { } } +fn build_gitignore(abs_path: &Path, fs: &dyn Fs) -> Result { + let contents = smol::block_on(fs.load(&abs_path))?; + let parent = abs_path.parent().unwrap_or(Path::new("/")); + let mut builder = GitignoreBuilder::new(parent); + for line in contents.lines() { + builder.add_line(Some(abs_path.into()), line)?; + } + Ok(builder.build()?) +} + +impl WorktreeId { + pub fn from_usize(handle_id: usize) -> Self { + Self(handle_id) + } + + pub(crate) fn from_proto(id: u64) -> Self { + Self(id as usize) + } + + pub fn to_proto(&self) -> u64 { + self.0 as u64 + } + + pub fn to_usize(&self) -> usize { + self.0 + } +} + +impl fmt::Display for WorktreeId { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.0.fmt(f) + } +} + +impl Deref for Worktree { + type Target = Snapshot; + + fn deref(&self) -> &Self::Target { + match self { + Worktree::Local(worktree) => &worktree.snapshot, + Worktree::Remote(worktree) => &worktree.snapshot, + } + } +} + +impl Deref for LocalWorktree { + type Target = Snapshot; + + fn deref(&self) -> &Self::Target { + &self.snapshot + } +} + +impl Deref for RemoteWorktree { + type Target = Snapshot; + + fn deref(&self) -> &Self::Target { + &self.snapshot + } +} + +impl fmt::Debug for LocalWorktree { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + self.snapshot.fmt(f) + } +} + impl fmt::Debug for Snapshot { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { for entry in self.entries_by_path.cursor::<()>() { From 34e42c0c5f3ce4c7cf7106409f3a8431551676c9 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 16:23:39 -0800 Subject: [PATCH 05/12] Remove worktree_id from buffer-related RPC messages --- crates/project/src/project.rs | 2 -- crates/project/src/worktree.rs | 21 ++++----------------- crates/rpc/proto/zed.proto | 19 +++++++------------ 3 files changed, 11 insertions(+), 31 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index efadbd707c1a466d2786f5fa04190ac47c43eb75..e161fcab91c9d32eaed454434205f4dd26e8fdaf 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -1504,7 +1504,6 @@ impl Project { .and_then(|shared_buffers| shared_buffers.get(&envelope.payload.buffer_id).cloned()) .ok_or_else(|| anyhow!("unknown buffer id {}", envelope.payload.buffer_id))?; let receipt = envelope.receipt(); - let worktree_id = envelope.payload.worktree_id; let buffer_id = envelope.payload.buffer_id; let save = cx.spawn(|_, mut cx| async move { buffer.update(&mut cx, |buffer, cx| buffer.save(cx)).await @@ -1519,7 +1518,6 @@ impl Project { receipt, proto::BufferSaved { project_id, - worktree_id, buffer_id, version: (&version).into(), mtime: Some(mtime.into()), diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 487411f97acd13b75e3e0508a3ccd38cd4da1915..58271d8fdc86cf9ba2eea08c3543f0a0ba64ce32 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -38,8 +38,7 @@ use std::{ }, time::{Duration, SystemTime}, }; -use sum_tree::{Bias, TreeMap}; -use sum_tree::{Edit, SeekTarget, SumTree}; +use sum_tree::{Bias, Edit, SeekTarget, SumTree, TreeMap}; use util::ResultExt; lazy_static! { @@ -398,22 +397,17 @@ impl Worktree { operation: Operation, cx: &mut ModelContext, ) { - if let Some((project_id, worktree_id, rpc)) = match self { + if let Some((project_id, rpc)) = match self { Worktree::Local(worktree) => worktree .share .as_ref() - .map(|share| (share.project_id, worktree.id(), worktree.client.clone())), - Worktree::Remote(worktree) => Some(( - worktree.project_id, - worktree.snapshot.id(), - worktree.client.clone(), - )), + .map(|share| (share.project_id, worktree.client.clone())), + Worktree::Remote(worktree) => Some((worktree.project_id, worktree.client.clone())), } { cx.spawn(|worktree, mut cx| async move { if let Err(error) = rpc .request(proto::UpdateBuffer { project_id, - worktree_id: worktree_id.0 as u64, buffer_id, operations: vec![language::proto::serialize_operation(&operation)], }) @@ -1408,7 +1402,6 @@ impl language::File for File { version: clock::Global, cx: &mut MutableAppContext, ) -> Task> { - let worktree_id = self.worktree.read(cx).id().to_proto(); self.worktree.update(cx, |worktree, cx| match worktree { Worktree::Local(worktree) => { let rpc = worktree.client.clone(); @@ -1419,7 +1412,6 @@ impl language::File for File { if let Some(project_id) = project_id { rpc.send(proto::BufferSaved { project_id, - worktree_id, buffer_id, version: (&version).into(), mtime: Some(entry.mtime.into()), @@ -1436,7 +1428,6 @@ impl language::File for File { let response = rpc .request(proto::SaveBuffer { project_id, - worktree_id, buffer_id, }) .await?; @@ -1467,14 +1458,12 @@ impl language::File for File { cx: &mut MutableAppContext, ) -> Option>> { let worktree = self.worktree.read(cx); - let worktree_id = worktree.id().to_proto(); let worktree = worktree.as_remote()?; let rpc = worktree.client.clone(); let project_id = worktree.project_id; Some(cx.foreground().spawn(async move { rpc.request(proto::FormatBuffer { project_id, - worktree_id, buffer_id, }) .await?; @@ -1492,14 +1481,12 @@ impl language::File for File { self.worktree.update(cx, |worktree, cx| { if let Worktree::Remote(worktree) = worktree { let project_id = worktree.project_id; - let worktree_id = worktree.id().to_proto(); let rpc = worktree.client.clone(); cx.background() .spawn(async move { if let Err(error) = rpc .send(proto::CloseBuffer { project_id, - worktree_id, buffer_id, }) .await diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 9891ce21cffafeaa3e7d622ac86d01a30f195cae..79e41f7704fea718f16fa62362df493849665a4f 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -144,35 +144,30 @@ message OpenBufferResponse { message CloseBuffer { uint64 project_id = 1; - uint64 worktree_id = 2; - uint64 buffer_id = 3; + uint64 buffer_id = 2; } message UpdateBuffer { uint64 project_id = 1; - uint64 worktree_id = 2; - uint64 buffer_id = 3; + uint64 buffer_id = 2; repeated Operation operations = 4; } message SaveBuffer { uint64 project_id = 1; - uint64 worktree_id = 2; - uint64 buffer_id = 3; + uint64 buffer_id = 2; } message BufferSaved { uint64 project_id = 1; - uint64 worktree_id = 2; - uint64 buffer_id = 3; - repeated VectorClockEntry version = 4; - Timestamp mtime = 5; + uint64 buffer_id = 2; + repeated VectorClockEntry version = 3; + Timestamp mtime = 4; } message FormatBuffer { uint64 project_id = 1; - uint64 worktree_id = 2; - uint64 buffer_id = 3; + uint64 buffer_id = 2; } message UpdateDiagnosticSummary { From f1fc0bde993e7d4a8b806fdf7c7c50b1b09c5fd8 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 17:43:24 -0800 Subject: [PATCH 06/12] Flush effects after every spawned future completes Co-Authored-By: Nathan Sobo --- crates/gpui/src/app.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/gpui/src/app.rs b/crates/gpui/src/app.rs index b35eda41fadea6110fa6986e5eb960db691e8c3d..11a8760700ba4323ad6ffe2ddc6ba7edf8b7d7ea 100644 --- a/crates/gpui/src/app.rs +++ b/crates/gpui/src/app.rs @@ -1653,8 +1653,13 @@ impl MutableAppContext { Fut: 'static + Future, T: 'static, { + let future = f(self.to_async()); let cx = self.to_async(); - self.foreground.spawn(f(cx)) + self.foreground.spawn(async move { + let result = future.await; + cx.0.borrow_mut().flush_effects(); + result + }) } pub fn to_async(&self) -> AsyncAppContext { From 2712cadaf69b2495fd8f4ebe91df63b3a73504a6 Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 17:44:24 -0800 Subject: [PATCH 07/12] Get integration tests passing * Fix misuse of guest buffer's id as its remote id Co-Authored-By: Nathan Sobo --- crates/project/src/project.rs | 12 ++++--- crates/project/src/worktree.rs | 12 +++---- crates/server/src/rpc.rs | 64 ++++++++++++---------------------- 3 files changed, 35 insertions(+), 53 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e161fcab91c9d32eaed454434205f4dd26e8fdaf..e7b221a985b9dcf85afcadd487216a36d8fcb681 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -427,7 +427,7 @@ impl Project { for worktree in this.worktrees(cx).collect::>() { worktree.update(cx, |worktree, cx| { let worktree = worktree.as_local_mut().unwrap(); - tasks.push(worktree.share(cx)); + tasks.push(worktree.share(project_id, cx)); }); } }); @@ -526,12 +526,14 @@ impl Project { cx.spawn(move |this, mut cx| async move { let load_result = load_buffer.await; - *tx.borrow_mut() = Some(this.update(&mut cx, |this, _| { + *tx.borrow_mut() = Some(this.update(&mut cx, |this, cx| { // Record the fact that the buffer is no longer loading. this.loading_buffers.remove(&path); let buffer = load_result.map_err(Arc::new)?; - this.open_buffers - .insert(buffer.id(), OpenBuffer::Loaded(buffer.downgrade())); + this.open_buffers.insert( + buffer.read(cx).remote_id() as usize, + OpenBuffer::Loaded(buffer.downgrade()), + ); Ok((buffer, Arc::new(AtomicBool::new(true)))) })); }) @@ -1109,7 +1111,7 @@ impl Project { if is_shared { worktree .update(&mut cx, |worktree, cx| { - worktree.as_local_mut().unwrap().share(cx) + worktree.as_local_mut().unwrap().share(project_id, cx) }) .await?; } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 58271d8fdc86cf9ba2eea08c3543f0a0ba64ce32..b364af2bf55c85db97eecc40323d3ffb647051c8 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -739,13 +739,11 @@ impl LocalWorktree { }) } - pub fn share(&mut self, cx: &mut ModelContext) -> Task> { - let project_id = if let Registration::Done { project_id } = self.registration { - project_id - } else { - return Task::ready(Err(anyhow!("cannot share worktree before registering it"))); - }; - + pub fn share( + &mut self, + project_id: u64, + cx: &mut ModelContext, + ) -> Task> { if self.share.is_some() { return Task::ready(Ok(())); } diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index cca6a3da4b0bff24c35e2bc7dcca8ba0997b2c49..8032a85eefb7364b767c3f17f414b85a13956008 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -1160,10 +1160,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); @@ -1298,10 +1296,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); worktree_a @@ -1400,10 +1396,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); worktree_a @@ -1470,7 +1464,9 @@ mod tests { .condition(&mut cx_a, |buf, _| buf.text() == "i-am-c, i-am-b, i-am-a") .await; buffer_b - .condition(&mut cx_b, |buf, _| buf.text() == "i-am-c, i-am-b, i-am-a") + .condition(&mut cx_b, |buf, _| { + dbg!(buf.text()) == "i-am-c, i-am-b, i-am-a" + }) .await; buffer_c .condition(&mut cx_c, |buf, _| buf.text() == "i-am-c, i-am-b, i-am-a") @@ -1550,10 +1546,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/dir", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) .await .unwrap(); worktree_a @@ -1645,10 +1639,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/dir", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) .await .unwrap(); worktree_a @@ -1725,10 +1717,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/dir", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) .await .unwrap(); worktree_a @@ -1801,10 +1791,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); worktree_a @@ -1890,10 +1878,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); worktree_a @@ -2110,10 +2096,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); worktree_a @@ -2617,10 +2601,8 @@ mod tests { cx, ) }); - let (worktree_a, _) = project_a - .update(&mut cx_a, |p, cx| { - p.find_or_create_worktree_for_abs_path("/a", false, cx) - }) + let worktree_a = project_a + .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) .await .unwrap(); worktree_a From 622aff3be2ec8aa3f0abbe4a7610903d3d1c27ad Mon Sep 17 00:00:00 2001 From: Max Brunsfeld Date: Fri, 21 Jan 2022 18:02:10 -0800 Subject: [PATCH 08/12] Get diagnostics crate's tests passing Update diagnostics on project instead of on worktree Co-Authored-By: Nathan Sobo --- crates/diagnostics/src/diagnostics.rs | 55 ++++++++++----------------- crates/project/src/project.rs | 37 +++++++++++------- 2 files changed, 44 insertions(+), 48 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index c1325c922e72a5c0931df8d7faa7af0c2654e2d2..88955255beb9497d60d89ba78e5ae14fba0412f7 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -725,7 +725,6 @@ mod tests { use gpui::TestAppContext; use language::{Diagnostic, DiagnosticEntry, DiagnosticSeverity, PointUtf16}; use serde_json::json; - use std::sync::Arc; use unindent::Unindent as _; use workspace::WorkspaceParams; @@ -764,21 +763,19 @@ mod tests { ) .await; - let worktree = project + project .update(&mut cx, |project, cx| { project.add_local_worktree("/test", false, cx) }) .await .unwrap(); - let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); // Create some diagnostics - worktree.update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .update_diagnostics( - Arc::from("/test/main.rs".as_ref()), + project.update(&mut cx, |project, cx| { + project + .update_diagnostic_entries( + PathBuf::from("/test/main.rs"), + None, vec![ DiagnosticEntry { range: PointUtf16::new(1, 8)..PointUtf16::new(1, 9), @@ -925,12 +922,12 @@ mod tests { }); // Diagnostics are added for another earlier path. - worktree.update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .update_diagnostics( - Arc::from("/test/consts.rs".as_ref()), + project.update(&mut cx, |project, cx| { + project.disk_based_diagnostics_started(cx); + project + .update_diagnostic_entries( + PathBuf::from("/test/consts.rs"), + None, vec![DiagnosticEntry { range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15), diagnostic: Diagnostic { @@ -945,13 +942,7 @@ mod tests { cx, ) .unwrap(); - }); - project.update(&mut cx, |_, cx| { - cx.emit(project::Event::DiagnosticsUpdated(ProjectPath { - worktree_id, - path: Arc::from("/test/consts.rs".as_ref()), - })); - cx.emit(project::Event::DiskBasedDiagnosticsFinished); + project.disk_based_diagnostics_finished(cx); }); view.next_notification(&cx).await; @@ -1030,12 +1021,12 @@ mod tests { }); // Diagnostics are added to the first path - worktree.update(&mut cx, |worktree, cx| { - worktree - .as_local_mut() - .unwrap() - .update_diagnostics( - Arc::from("/test/consts.rs".as_ref()), + project.update(&mut cx, |project, cx| { + project.disk_based_diagnostics_started(cx); + project + .update_diagnostic_entries( + PathBuf::from("/test/consts.rs"), + None, vec![ DiagnosticEntry { range: PointUtf16::new(0, 15)..PointUtf16::new(0, 15), @@ -1064,13 +1055,7 @@ mod tests { cx, ) .unwrap(); - }); - project.update(&mut cx, |_, cx| { - cx.emit(project::Event::DiagnosticsUpdated(ProjectPath { - worktree_id, - path: Arc::from("/test/consts.rs".as_ref()), - })); - cx.emit(project::Event::DiskBasedDiagnosticsFinished); + project.disk_based_diagnostics_finished(cx); }); view.next_notification(&cx).await; diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index e7b221a985b9dcf85afcadd487216a36d8fcb681..bceb1511166ca49c3b40069bfea7dbbe96fd9568 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -14,7 +14,7 @@ use gpui::{ }; use language::{ range_from_lsp, Bias, Buffer, Diagnostic, DiagnosticEntry, File as _, Language, - LanguageRegistry, Operation, ToOffset, ToPointUtf16, + LanguageRegistry, Operation, PointUtf16, ToOffset, ToPointUtf16, }; use lsp::{DiagnosticSeverity, LanguageServer}; use postage::{prelude::Stream, watch}; @@ -787,17 +787,10 @@ impl Project { disk_based_sources: &HashSet, cx: &mut ModelContext, ) -> Result<()> { - let path = params + let abs_path = params .uri .to_file_path() .map_err(|_| anyhow!("URI is not a file"))?; - let (worktree, relative_path) = self - .find_worktree_for_abs_path(&path, cx) - .ok_or_else(|| anyhow!("no worktree found for diagnostics"))?; - let project_path = ProjectPath { - worktree_id: worktree.read(cx).id(), - path: relative_path.into(), - }; let mut next_group_id = 0; let mut diagnostics = Vec::default(); let mut primary_diagnostic_group_ids = HashMap::default(); @@ -885,6 +878,25 @@ impl Project { } } + self.update_diagnostic_entries(abs_path, params.version, diagnostics, cx)?; + Ok(()) + } + + pub fn update_diagnostic_entries( + &mut self, + abs_path: PathBuf, + version: Option, + diagnostics: Vec>, + cx: &mut ModelContext, + ) -> Result<(), anyhow::Error> { + let (worktree, relative_path) = self + .find_worktree_for_abs_path(&abs_path, cx) + .ok_or_else(|| anyhow!("no worktree found for diagnostics"))?; + let project_path = ProjectPath { + worktree_id: worktree.read(cx).id(), + path: relative_path.into(), + }; + for buffer in self.open_buffers.values() { if let Some(buffer) = buffer.upgrade(cx) { if buffer @@ -893,13 +905,12 @@ impl Project { .map_or(false, |file| *file.path() == project_path.path) { buffer.update(cx, |buffer, cx| { - buffer.update_diagnostics(params.version, diagnostics.clone(), cx) + buffer.update_diagnostics(version, diagnostics.clone(), cx) })?; break; } } } - worktree.update(cx, |worktree, cx| { worktree .as_local_mut() @@ -1268,14 +1279,14 @@ impl Project { }) } - fn disk_based_diagnostics_started(&mut self, cx: &mut ModelContext) { + pub fn disk_based_diagnostics_started(&mut self, cx: &mut ModelContext) { self.language_servers_with_diagnostics_running += 1; if self.language_servers_with_diagnostics_running == 1 { cx.emit(Event::DiskBasedDiagnosticsStarted); } } - fn disk_based_diagnostics_finished(&mut self, cx: &mut ModelContext) { + pub fn disk_based_diagnostics_finished(&mut self, cx: &mut ModelContext) { cx.emit(Event::DiskBasedDiagnosticsUpdated); self.language_servers_with_diagnostics_running -= 1; if self.language_servers_with_diagnostics_running == 0 { From 31dfd01fdaeaae638ea64d6494394f04282e182b Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 22 Jan 2022 11:14:50 +0100 Subject: [PATCH 09/12] Make `add_local_worktree` private and use `find_or_create_local_worktree` The former always adds a worktree, even if we have one already in the project and that could be misused. The public API should always search for a local worktree containing the requested path first so that the project can uphold invariants about which worktrees it has. --- crates/diagnostics/src/diagnostics.rs | 2 +- crates/file_finder/src/file_finder.rs | 10 +-- crates/project/src/project.rs | 87 ++++++++++++----------- crates/project_panel/src/project_panel.rs | 4 +- crates/server/src/rpc.rs | 60 ++++++++++------ crates/workspace/src/workspace.rs | 2 +- crates/zed/src/zed.rs | 12 ++-- 7 files changed, 98 insertions(+), 79 deletions(-) diff --git a/crates/diagnostics/src/diagnostics.rs b/crates/diagnostics/src/diagnostics.rs index 88955255beb9497d60d89ba78e5ae14fba0412f7..eedb77c9be1fae401b1dfdf829fec775c501aa49 100644 --- a/crates/diagnostics/src/diagnostics.rs +++ b/crates/diagnostics/src/diagnostics.rs @@ -765,7 +765,7 @@ mod tests { project .update(&mut cx, |project, cx| { - project.add_local_worktree("/test", false, cx) + project.find_or_create_local_worktree("/test", false, cx) }) .await .unwrap(); diff --git a/crates/file_finder/src/file_finder.rs b/crates/file_finder/src/file_finder.rs index 99a60cf28d04af95abc747887a798b52d8d61093..588a80593e2611b07cc70ea788514c244218e976 100644 --- a/crates/file_finder/src/file_finder.rs +++ b/crates/file_finder/src/file_finder.rs @@ -457,7 +457,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -518,7 +518,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/dir"), false, cx) + project.find_or_create_local_worktree("/dir", false, cx) }) .await .unwrap(); @@ -584,11 +584,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path( - Path::new("/root/the-parent-dir/the-file"), - false, - cx, - ) + project.find_or_create_local_worktree("/root/the-parent-dir/the-file", false, cx) }) .await .unwrap(); diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index bceb1511166ca49c3b40069bfea7dbbe96fd9568..d0b42eaaafa9d15dd9e09a513bbb6327ded749ba 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -568,7 +568,7 @@ impl Project { abs_path: PathBuf, cx: &mut ModelContext, ) -> Task> { - let worktree_task = self.find_or_create_worktree_for_abs_path(&abs_path, false, cx); + let worktree_task = self.find_or_create_local_worktree(&abs_path, false, cx); cx.spawn(|this, mut cx| async move { let (worktree, path) = worktree_task.await?; worktree @@ -890,7 +890,7 @@ impl Project { cx: &mut ModelContext, ) -> Result<(), anyhow::Error> { let (worktree, relative_path) = self - .find_worktree_for_abs_path(&abs_path, cx) + .find_local_worktree(&abs_path, cx) .ok_or_else(|| anyhow!("no worktree found for diagnostics"))?; let project_path = ProjectPath { worktree_id: worktree.read(cx).id(), @@ -995,15 +995,14 @@ impl Project { .to_file_path() .map_err(|_| anyhow!("invalid target path"))?; - let (worktree, relative_path) = if let Some(result) = this - .read_with(&cx, |this, cx| { - this.find_worktree_for_abs_path(&abs_path, cx) - }) { + let (worktree, relative_path) = if let Some(result) = + this.read_with(&cx, |this, cx| this.find_local_worktree(&abs_path, cx)) + { result } else { - let (worktree, relative_path) = this + let worktree = this .update(&mut cx, |this, cx| { - this.create_worktree_for_abs_path(&abs_path, true, cx) + this.create_local_worktree(&abs_path, true, cx) }) .await?; this.update(&mut cx, |this, cx| { @@ -1012,7 +1011,7 @@ impl Project { lang_server.clone(), ); }); - (worktree, relative_path) + (worktree, PathBuf::new()) }; let project_path = ProjectPath { @@ -1045,34 +1044,23 @@ impl Project { } } - pub fn find_or_create_worktree_for_abs_path( + pub fn find_or_create_local_worktree( &self, abs_path: impl AsRef, weak: bool, cx: &mut ModelContext, ) -> Task, PathBuf)>> { let abs_path = abs_path.as_ref(); - if let Some((tree, relative_path)) = self.find_worktree_for_abs_path(abs_path, cx) { + if let Some((tree, relative_path)) = self.find_local_worktree(abs_path, cx) { Task::ready(Ok((tree.clone(), relative_path.into()))) } else { - self.create_worktree_for_abs_path(abs_path, weak, cx) + let worktree = self.create_local_worktree(abs_path, weak, cx); + cx.foreground() + .spawn(async move { Ok((worktree.await?, PathBuf::new())) }) } } - fn create_worktree_for_abs_path( - &self, - abs_path: &Path, - weak: bool, - cx: &mut ModelContext, - ) -> Task, PathBuf)>> { - let worktree = self.add_local_worktree(abs_path, weak, cx); - cx.background().spawn(async move { - let worktree = worktree.await?; - Ok((worktree, PathBuf::new())) - }) - } - - fn find_worktree_for_abs_path( + fn find_local_worktree( &self, abs_path: &Path, cx: &AppContext, @@ -1096,7 +1084,7 @@ impl Project { } } - pub fn add_local_worktree( + fn create_local_worktree( &self, abs_path: impl AsRef, weak: bool, @@ -1909,7 +1897,7 @@ mod tests { let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(&root_link_path, false, cx) + project.find_or_create_local_worktree(&root_link_path, false, cx) }) .await .unwrap(); @@ -1984,7 +1972,7 @@ mod tests { let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(dir.path(), false, cx) + project.find_or_create_local_worktree(dir.path(), false, cx) }) .await .unwrap(); @@ -2090,7 +2078,7 @@ mod tests { let project = build_project(Arc::new(RealFs), &mut cx); let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(&dir.path(), false, cx) + project.find_or_create_local_worktree(&dir.path(), false, cx) }) .await .unwrap(); @@ -2144,7 +2132,7 @@ mod tests { let (tree, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(dir.path().join("b.rs"), false, cx) + project.find_or_create_local_worktree(dir.path().join("b.rs"), false, cx) }) .await .unwrap(); @@ -2241,9 +2229,12 @@ mod tests { let project = build_project(fs.clone(), &mut cx); let worktree_id = project - .update(&mut cx, |p, cx| p.add_local_worktree("/dir", false, cx)) + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap() + .0 .read_with(&cx, |tree, _| tree.id()); let buffer = project @@ -2277,10 +2268,11 @@ mod tests { let project = build_project(fs.clone(), &mut cx); let worktree_id = project .update(&mut cx, |p, cx| { - p.add_local_worktree("/dir/file1", false, cx) + p.find_or_create_local_worktree("/dir/file1", false, cx) }) .await .unwrap() + .0 .read_with(&cx, |tree, _| tree.id()); let buffer = project @@ -2318,8 +2310,10 @@ mod tests { let project = build_project(Arc::new(RealFs), &mut cx); let rpc = project.read_with(&cx, |p, _| p.client.clone()); - let tree = project - .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + let (tree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree(dir.path(), false, cx) + }) .await .unwrap(); let worktree_id = tree.read_with(&cx, |tree, _| tree.id()); @@ -2460,9 +2454,12 @@ mod tests { let project = build_project(fs.clone(), &mut cx); let worktree_id = project - .update(&mut cx, |p, cx| p.add_local_worktree("/the-dir", false, cx)) + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree("/the-dir", false, cx) + }) .await .unwrap() + .0 .read_with(&cx, |tree, _| tree.id()); // Spawn multiple tasks to open paths, repeating some paths. @@ -2506,8 +2503,10 @@ mod tests { })); let project = build_project(Arc::new(RealFs), &mut cx); - let worktree = project - .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + let (worktree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree(dir.path(), false, cx) + }) .await .unwrap(); let worktree_id = worktree.read_with(&cx, |worktree, _| worktree.id()); @@ -2638,8 +2637,10 @@ mod tests { let dir = temp_tree(json!({ "the-file": initial_contents })); let project = build_project(Arc::new(RealFs), &mut cx); - let worktree = project - .update(&mut cx, |p, cx| p.add_local_worktree(dir.path(), false, cx)) + let (worktree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree(dir.path(), false, cx) + }) .await .unwrap(); let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); @@ -2747,8 +2748,10 @@ mod tests { .await; let project = build_project(fs.clone(), &mut cx); - let worktree = project - .update(&mut cx, |p, cx| p.add_local_worktree("/the-dir", false, cx)) + let (worktree, _) = project + .update(&mut cx, |p, cx| { + p.find_or_create_local_worktree("/the-dir", false, cx) + }) .await .unwrap(); let worktree_id = worktree.read_with(&cx, |tree, _| tree.id()); diff --git a/crates/project_panel/src/project_panel.rs b/crates/project_panel/src/project_panel.rs index 781c4cd586a88300b5aebc8e6885600557b82ac9..643891f5d4c2d5735c322c3007aa5c4742985e8c 100644 --- a/crates/project_panel/src/project_panel.rs +++ b/crates/project_panel/src/project_panel.rs @@ -644,7 +644,7 @@ mod tests { }); let (root1, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path("/root1", false, cx) + project.find_or_create_local_worktree("/root1", false, cx) }) .await .unwrap(); @@ -653,7 +653,7 @@ mod tests { .await; let (root2, _) = project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path("/root2", false, cx) + project.find_or_create_local_worktree("/root2", false, cx) }) .await .unwrap(); diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 8032a85eefb7364b767c3f17f414b85a13956008..4ad23826999063335669da3e8d829ea4bc917dee 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -1160,8 +1160,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); let worktree_id = worktree_a.read_with(&cx_a, |tree, _| tree.id()); @@ -1296,8 +1298,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -1396,8 +1400,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -1546,8 +1552,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap(); worktree_a @@ -1639,8 +1647,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap(); worktree_a @@ -1717,8 +1727,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/dir", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/dir", false, cx) + }) .await .unwrap(); worktree_a @@ -1791,8 +1803,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -1878,8 +1892,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -2096,8 +2112,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a @@ -2601,8 +2619,10 @@ mod tests { cx, ) }); - let worktree_a = project_a - .update(&mut cx_a, |p, cx| p.add_local_worktree("/a", false, cx)) + let (worktree_a, _) = project_a + .update(&mut cx_a, |p, cx| { + p.find_or_create_local_worktree("/a", false, cx) + }) .await .unwrap(); worktree_a diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index ef91d4b015c8dfdc47fad81550cb9e1f7321f37f..8dc98c852bd17bca7bb1f2839bf2b83e0b93c47d 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -723,7 +723,7 @@ impl Workspace { cx: &mut ViewContext, ) -> Task> { let entry = self.project().update(cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(abs_path, false, cx) + project.find_or_create_local_worktree(abs_path, false, cx) }); cx.spawn(|_, cx| async move { let (worktree, path) = entry.await?; diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 5f5422d25671708138fcc7ee697f4fd0203bfdd0..fc7ae9a3c5fce93dea530654cebb00b375474aaa 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -245,7 +245,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -358,7 +358,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/dir1"), false, cx) + project.find_or_create_local_worktree("/dir1", false, cx) }) .await .unwrap(); @@ -425,7 +425,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -474,7 +474,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -626,7 +626,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); @@ -689,7 +689,7 @@ mod tests { params .project .update(&mut cx, |project, cx| { - project.find_or_create_worktree_for_abs_path(Path::new("/root"), false, cx) + project.find_or_create_local_worktree("/root", false, cx) }) .await .unwrap(); From 2773cab4ec1ae5da621f5aa79d44e5e0a758ff5f Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Sat, 22 Jan 2022 11:34:44 +0100 Subject: [PATCH 10/12] Simplify opening buffers in the Project and assign language synchronously --- crates/project/src/project.rs | 43 ++++++++++++----------------------- 1 file changed, 15 insertions(+), 28 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index d0b42eaaafa9d15dd9e09a513bbb6327ded749ba..c036ded150cbc4e7527d3fc573272822cde7f597 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -23,10 +23,7 @@ use std::{ convert::TryInto, ops::Range, path::{Path, PathBuf}, - sync::{ - atomic::{AtomicBool, Ordering::SeqCst}, - Arc, - }, + sync::{atomic::AtomicBool, Arc}, }; use util::{post_inc, ResultExt, TryFutureExt as _}; @@ -48,9 +45,7 @@ pub struct Project { open_buffers: HashMap, loading_buffers: HashMap< ProjectPath, - postage::watch::Receiver< - Option, Arc), Arc>>, - >, + postage::watch::Receiver, Arc>>>, >, shared_buffers: HashMap>>, } @@ -498,15 +493,13 @@ impl Project { let worktree = if let Some(worktree) = self.worktree_for_id(path.worktree_id, cx) { worktree } else { - return cx - .foreground() - .spawn(async move { Err(anyhow!("no such worktree")) }); + return Task::ready(Err(anyhow!("no such worktree"))); }; // If there is already a buffer for the given path, then return it. let existing_buffer = self.get_open_buffer(&path, cx); if let Some(existing_buffer) = existing_buffer { - return cx.foreground().spawn(async move { Ok(existing_buffer) }); + return Task::ready(Ok(existing_buffer)); } let mut loading_watch = match self.loading_buffers.entry(path.clone()) { @@ -534,7 +527,8 @@ impl Project { buffer.read(cx).remote_id() as usize, OpenBuffer::Loaded(buffer.downgrade()), ); - Ok((buffer, Arc::new(AtomicBool::new(true)))) + this.assign_language_to_buffer(&worktree, &buffer, cx); + Ok(buffer) })); }) .detach(); @@ -542,23 +536,16 @@ impl Project { } }; - cx.spawn(|this, mut cx| async move { - let (buffer, buffer_is_new) = loop { + cx.foreground().spawn(async move { + loop { if let Some(result) = loading_watch.borrow().as_ref() { - break match result { - Ok((buf, is_new)) => Ok((buf.clone(), is_new.fetch_and(false, SeqCst))), - Err(error) => Err(anyhow!("{}", error)), - }; + match result { + Ok(buffer) => return Ok(buffer.clone()), + Err(error) => return Err(anyhow!("{}", error)), + } } loading_watch.recv().await; - }?; - - if buffer_is_new { - this.update(&mut cx, |this, cx| { - this.assign_language_to_buffer(worktree, buffer.clone(), cx) - }); } - Ok(buffer) }) } @@ -582,7 +569,7 @@ impl Project { this.update(&mut cx, |this, cx| { this.open_buffers .insert(buffer.id(), OpenBuffer::Loaded(buffer.downgrade())); - this.assign_language_to_buffer(worktree, buffer, cx) + this.assign_language_to_buffer(&worktree, &buffer, cx); }); Ok(()) }) @@ -632,8 +619,8 @@ impl Project { fn assign_language_to_buffer( &mut self, - worktree: ModelHandle, - buffer: ModelHandle, + worktree: &ModelHandle, + buffer: &ModelHandle, cx: &mut ModelContext, ) -> Option<()> { // Set the buffer's language From 8bf628c17b2abe995eeda643e29a71e92f529875 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 08:46:37 -0700 Subject: [PATCH 11/12] =?UTF-8?q?Update=20new=20buffers=20with=20existing?= =?UTF-8?q?=20diagnostics=20in=20Project=20=E2=80=93=20after=20assigning?= =?UTF-8?q?=20language?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/project/src/project.rs | 62 +++++++++++++++++++++------------- crates/project/src/worktree.rs | 32 ++++++++++-------- 2 files changed, 56 insertions(+), 38 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index c036ded150cbc4e7527d3fc573272822cde7f597..5faa830df6ee8764703560fea9a963e8e7ed4fc0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -489,20 +489,20 @@ impl Project { path: impl Into, cx: &mut ModelContext, ) -> Task>> { - let path = path.into(); - let worktree = if let Some(worktree) = self.worktree_for_id(path.worktree_id, cx) { + let project_path = path.into(); + let worktree = if let Some(worktree) = self.worktree_for_id(project_path.worktree_id, cx) { worktree } else { return Task::ready(Err(anyhow!("no such worktree"))); }; // If there is already a buffer for the given path, then return it. - let existing_buffer = self.get_open_buffer(&path, cx); + let existing_buffer = self.get_open_buffer(&project_path, cx); if let Some(existing_buffer) = existing_buffer { return Task::ready(Ok(existing_buffer)); } - let mut loading_watch = match self.loading_buffers.entry(path.clone()) { + let mut loading_watch = match self.loading_buffers.entry(project_path.clone()) { // If the given path is already being loaded, then wait for that existing // task to complete and return the same buffer. hash_map::Entry::Occupied(e) => e.get().clone(), @@ -512,16 +512,15 @@ impl Project { let (mut tx, rx) = postage::watch::channel(); entry.insert(rx.clone()); - let load_buffer = worktree.update(cx, |worktree, cx| match worktree { - Worktree::Local(worktree) => worktree.open_buffer(&path.path, cx), - Worktree::Remote(worktree) => worktree.open_buffer(&path.path, cx), + let load_buffer = worktree.update(cx, |worktree, cx| { + worktree.load_buffer(&project_path.path, cx) }); cx.spawn(move |this, mut cx| async move { let load_result = load_buffer.await; *tx.borrow_mut() = Some(this.update(&mut cx, |this, cx| { // Record the fact that the buffer is no longer loading. - this.loading_buffers.remove(&path); + this.loading_buffers.remove(&project_path); let buffer = load_result.map_err(Arc::new)?; this.open_buffers.insert( buffer.read(cx).remote_id() as usize, @@ -623,31 +622,46 @@ impl Project { buffer: &ModelHandle, cx: &mut ModelContext, ) -> Option<()> { + let (path, full_path) = { + let file = buffer.read(cx).file()?; + (file.path().clone(), file.full_path()) + }; + // Set the buffer's language - let full_path = buffer.read(cx).file()?.full_path(); let language = self.languages.select_language(&full_path)?.clone(); buffer.update(cx, |buffer, cx| { buffer.set_language(Some(language.clone()), cx); }); // For local worktrees, start a language server if needed. + // Also assign the language server and any previously stored diagnostics to the buffer. let worktree = worktree.read(cx); - let worktree_id = worktree.id(); - let worktree_abs_path = worktree.as_local()?.abs_path().clone(); - let language_server = match self - .language_servers - .entry((worktree_id, language.name().to_string())) - { - hash_map::Entry::Occupied(e) => Some(e.get().clone()), - hash_map::Entry::Vacant(e) => { - Self::start_language_server(self.client.clone(), language, &worktree_abs_path, cx) - .map(|server| e.insert(server).clone()) - } - }; + if let Some(local_worktree) = worktree.as_local() { + let worktree_id = local_worktree.id(); + let diagnostics = local_worktree.diagnostics_for_path(&path); + let worktree_abs_path = local_worktree.abs_path().clone(); + + let language_server = match self + .language_servers + .entry((worktree_id, language.name().to_string())) + { + hash_map::Entry::Occupied(e) => Some(e.get().clone()), + hash_map::Entry::Vacant(e) => Self::start_language_server( + self.client.clone(), + language, + &worktree_abs_path, + cx, + ) + .map(|server| e.insert(server).clone()), + }; - buffer.update(cx, |buffer, cx| { - buffer.set_language_server(language_server, cx) - }); + buffer.update(cx, |buffer, cx| { + buffer.set_language_server(language_server, cx); + if let Some(diagnostics) = diagnostics { + buffer.update_diagnostics(None, diagnostics, cx).log_err(); + } + }); + } None } diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index b364af2bf55c85db97eecc40323d3ffb647051c8..719049ce308cb9c32cce31f80f8c50ba61d65cbc 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -347,6 +347,17 @@ impl Worktree { } } + pub fn load_buffer( + &mut self, + path: &Path, + cx: &mut ModelContext, + ) -> Task>> { + match self { + Worktree::Local(worktree) => worktree.load_buffer(path, cx), + Worktree::Remote(worktree) => worktree.load_buffer(path, cx), + } + } + pub fn diagnostic_summaries<'a>( &'a self, ) -> impl Iterator, DiagnosticSummary)> + 'a { @@ -536,7 +547,7 @@ impl LocalWorktree { self.config.collaborators.clone() } - pub(crate) fn open_buffer( + pub(crate) fn load_buffer( &mut self, path: &Path, cx: &mut ModelContext, @@ -546,21 +557,14 @@ impl LocalWorktree { let (file, contents) = this .update(&mut cx, |t, cx| t.as_local().unwrap().load(&path, cx)) .await?; - - let diagnostics = this.update(&mut cx, |this, _| { - this.as_local_mut().unwrap().diagnostics.get(&path).cloned() - }); - - Ok(cx.add_model(|cx| { - let mut buffer = Buffer::from_file(0, contents, Box::new(file), cx); - if let Some(diagnostics) = diagnostics { - buffer.update_diagnostics(None, diagnostics, cx).unwrap(); - } - buffer - })) + Ok(cx.add_model(|cx| Buffer::from_file(0, contents, Box::new(file), cx))) }) } + pub fn diagnostics_for_path(&self, path: &Path) -> Option>> { + self.diagnostics.get(path).cloned() + } + pub fn update_diagnostics( &mut self, worktree_path: Arc, @@ -798,7 +802,7 @@ impl LocalWorktree { } impl RemoteWorktree { - pub(crate) fn open_buffer( + pub(crate) fn load_buffer( &mut self, path: &Path, cx: &mut ModelContext, From 83418204b6757a604cb5e6d71e842a7c297960c6 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 09:54:25 -0700 Subject: [PATCH 12/12] Assign diagnostics on buffer even if it doesn't have a language This shouldn't be necessary in practice but makes testing easier. --- crates/language/src/buffer.rs | 26 +++++++------- crates/project/src/project.rs | 66 +++++++++++++++++++---------------- 2 files changed, 48 insertions(+), 44 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 0c97179e6f1ef949c965ed2ff267eec9a5e2c851..f9f0192d9be9d48bb2cf172c3cc490d827814837 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -869,19 +869,19 @@ impl Buffer { } let version = version.map(|version| version as usize); - let content = if let Some(version) = version { - let language_server = self.language_server.as_mut().unwrap(); - language_server - .pending_snapshots - .retain(|&v, _| v >= version); - let snapshot = language_server - .pending_snapshots - .get(&version) - .ok_or_else(|| anyhow!("missing snapshot"))?; - &snapshot.buffer_snapshot - } else { - self.deref() - }; + let content = + if let Some((version, language_server)) = version.zip(self.language_server.as_mut()) { + language_server + .pending_snapshots + .retain(|&v, _| v >= version); + let snapshot = language_server + .pending_snapshots + .get(&version) + .ok_or_else(|| anyhow!("missing snapshot"))?; + &snapshot.buffer_snapshot + } else { + self.deref() + }; diagnostics.sort_unstable_by(|a, b| { Ordering::Equal diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 5faa830df6ee8764703560fea9a963e8e7ed4fc0..cd9ea34c3b960fb781fa7d2d495014b69bb2895f 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -627,40 +627,44 @@ impl Project { (file.path().clone(), file.full_path()) }; - // Set the buffer's language - let language = self.languages.select_language(&full_path)?.clone(); - buffer.update(cx, |buffer, cx| { - buffer.set_language(Some(language.clone()), cx); - }); + // If the buffer has a language, set it and start/assign the language server + if let Some(language) = self.languages.select_language(&full_path) { + buffer.update(cx, |buffer, cx| { + buffer.set_language(Some(language.clone()), cx); + }); - // For local worktrees, start a language server if needed. - // Also assign the language server and any previously stored diagnostics to the buffer. - let worktree = worktree.read(cx); - if let Some(local_worktree) = worktree.as_local() { - let worktree_id = local_worktree.id(); - let diagnostics = local_worktree.diagnostics_for_path(&path); - let worktree_abs_path = local_worktree.abs_path().clone(); - - let language_server = match self - .language_servers - .entry((worktree_id, language.name().to_string())) - { - hash_map::Entry::Occupied(e) => Some(e.get().clone()), - hash_map::Entry::Vacant(e) => Self::start_language_server( - self.client.clone(), - language, - &worktree_abs_path, - cx, - ) - .map(|server| e.insert(server).clone()), - }; + // For local worktrees, start a language server if needed. + // Also assign the language server and any previously stored diagnostics to the buffer. + if let Some(local_worktree) = worktree.read(cx).as_local() { + let worktree_id = local_worktree.id(); + let worktree_abs_path = local_worktree.abs_path().clone(); - buffer.update(cx, |buffer, cx| { - buffer.set_language_server(language_server, cx); - if let Some(diagnostics) = diagnostics { + let language_server = match self + .language_servers + .entry((worktree_id, language.name().to_string())) + { + hash_map::Entry::Occupied(e) => Some(e.get().clone()), + hash_map::Entry::Vacant(e) => Self::start_language_server( + self.client.clone(), + language.clone(), + &worktree_abs_path, + cx, + ) + .map(|server| e.insert(server).clone()), + }; + + buffer.update(cx, |buffer, cx| { + buffer.set_language_server(language_server, cx); + }); + } + } + + if let Some(local_worktree) = worktree.read(cx).as_local() { + if let Some(diagnostics) = local_worktree.diagnostics_for_path(&path) { + buffer.update(cx, |buffer, cx| { buffer.update_diagnostics(None, diagnostics, cx).log_err(); - } - }); + }); + } } None