From e2a2073bdbb839efb3d5619b52ecaa885df797be Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 14:29:36 -0700 Subject: [PATCH 1/8] Remove worktree_path from File struct --- crates/editor/src/items.rs | 2 +- crates/language/src/buffer.rs | 18 ++++++++-------- crates/project/src/project.rs | 10 +++------ crates/project/src/worktree.rs | 38 ++++++++++++++-------------------- crates/rpc/proto/zed.proto | 7 +++++++ 5 files changed, 35 insertions(+), 40 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index dd4b162027..78f5bcc827 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -126,7 +126,7 @@ impl ItemView for Editor { .buffer() .read(cx) .file(cx) - .and_then(|file| file.file_name()); + .and_then(|file| file.file_name(cx)); if let Some(name) = filename { name.to_string_lossy().into() } else { diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index f9f0192d9b..cceef3992e 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -162,15 +162,15 @@ pub trait File { fn path(&self) -> &Arc; /// Returns the absolute path of this file. - fn abs_path(&self) -> Option; + fn abs_path(&self, cx: &AppContext) -> Option; /// Returns the path of this file relative to the worktree's parent directory (this means it /// includes the name of the worktree's root folder). - fn full_path(&self) -> PathBuf; + fn full_path(&self, cx: &AppContext) -> PathBuf; /// Returns the last component of this handle's absolute path. If this handle refers to the root /// of its worktree, then this method will return the name of the worktree itself. - fn file_name(&self) -> Option; + fn file_name(&self, cx: &AppContext) -> Option; fn is_deleted(&self) -> bool; @@ -457,7 +457,7 @@ impl Buffer { if let Some(LanguageServerState { server, .. }) = self.language_server.as_ref() { let server = server.clone(); - let abs_path = file.abs_path().unwrap(); + let abs_path = file.abs_path(cx).unwrap(); let version = self.version(); cx.spawn(|this, mut cx| async move { let edits = server @@ -619,7 +619,7 @@ impl Buffer { None }; - self.update_language_server(); + self.update_language_server(cx); } pub fn did_save( @@ -643,7 +643,7 @@ impl Buffer { lsp::DidSaveTextDocumentParams { text_document: lsp::TextDocumentIdentifier { uri: lsp::Url::from_file_path( - self.file.as_ref().unwrap().abs_path().unwrap(), + self.file.as_ref().unwrap().abs_path(cx).unwrap(), ) .unwrap(), }, @@ -1226,7 +1226,7 @@ impl Buffer { self.set_active_selections(Arc::from([]), cx); } - fn update_language_server(&mut self) { + fn update_language_server(&mut self, cx: &AppContext) { let language_server = if let Some(language_server) = self.language_server.as_mut() { language_server } else { @@ -1236,7 +1236,7 @@ impl Buffer { .file .as_ref() .map_or(Path::new("/").to_path_buf(), |file| { - file.abs_path().unwrap() + file.abs_path(cx).unwrap() }); let version = post_inc(&mut language_server.next_version); @@ -1381,7 +1381,7 @@ impl Buffer { } self.reparse(cx); - self.update_language_server(); + self.update_language_server(cx); cx.emit(Event::Edited); if !was_dirty { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index cd9ea34c3b..b540d9713a 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -624,7 +624,7 @@ impl Project { ) -> Option<()> { let (path, full_path) = { let file = buffer.read(cx).file()?; - (file.path().clone(), file.full_path()) + (file.path().clone(), file.full_path(cx)) }; // If the buffer has a language, set it and start/assign the language server @@ -938,7 +938,7 @@ impl Project { let buffer_abs_path; if let Some(file) = File::from_dyn(buffer.file()) { worktree = file.worktree.clone(); - buffer_abs_path = file.abs_path(); + buffer_abs_path = file.abs_path(cx); } else { return Task::ready(Err(anyhow!("buffer does not belong to any worktree"))); }; @@ -1168,7 +1168,6 @@ impl Project { ) { 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 { @@ -1185,7 +1184,6 @@ impl Project { { File { is_local: local, - worktree_path: worktree_path.clone(), entry_id: Some(entry.id), mtime: entry.mtime, path: entry.path.clone(), @@ -1196,7 +1194,6 @@ impl Project { { File { is_local: local, - worktree_path: worktree_path.clone(), entry_id: Some(entry.id), mtime: entry.mtime, path: entry.path.clone(), @@ -1205,7 +1202,6 @@ impl Project { } else { File { is_local: local, - worktree_path: worktree_path.clone(), entry_id: None, path: old_file.path().clone(), mtime: old_file.mtime(), @@ -2185,7 +2181,7 @@ mod tests { cx.update(|cx| { let target_buffer = definition.target_buffer.read(cx); assert_eq!( - target_buffer.file().unwrap().abs_path(), + target_buffer.file().unwrap().abs_path(cx), Some(dir.path().join("a.rs")) ); assert_eq!(definition.target_range.to_offset(target_buffer), 9..10); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 719049ce30..ea87a14341 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -631,7 +631,6 @@ impl LocalWorktree { fn load(&self, path: &Path, cx: &mut ModelContext) -> Task> { let handle = cx.handle(); let path = Arc::from(path); - let worktree_path = self.abs_path.clone(); let abs_path = self.absolutize(&path); let background_snapshot = self.background_snapshot.clone(); let fs = self.fs.clone(); @@ -644,7 +643,6 @@ impl LocalWorktree { File { entry_id: Some(entry.id), worktree: handle, - worktree_path, path: entry.path, mtime: entry.mtime, is_local: true, @@ -664,19 +662,16 @@ impl LocalWorktree { let text = buffer.as_rope().clone(); let version = buffer.version(); let save = self.save(path, text, cx); - cx.spawn(|this, mut cx| async move { + let handle = cx.handle(); + cx.as_mut().spawn(|mut cx| async move { let entry = save.await?; - let file = this.update(&mut cx, |this, cx| { - let this = this.as_local_mut().unwrap(); - File { - entry_id: Some(entry.id), - worktree: cx.handle(), - worktree_path: this.abs_path.clone(), - path: entry.path, - mtime: entry.mtime, - is_local: true, - } - }); + let file = File { + entry_id: Some(entry.id), + worktree: handle, + path: entry.path, + mtime: entry.mtime, + is_local: true, + }; buffer_handle.update(&mut cx, |buffer, cx| { buffer.did_save(version, file.mtime, Some(Box::new(file)), cx); @@ -811,7 +806,6 @@ impl RemoteWorktree { let replica_id = self.replica_id; let project_id = self.project_id; let remote_worktree_id = self.id(); - let root_path = self.snapshot.abs_path.clone(); let path: Arc = Arc::from(path); let path_string = path.to_string_lossy().to_string(); cx.spawn_weak(move |this, mut cx| async move { @@ -834,7 +828,6 @@ impl RemoteWorktree { let file = File { entry_id: Some(entry.id), worktree: this.clone(), - worktree_path: root_path, path: entry.path, mtime: entry.mtime, is_local: false, @@ -1354,7 +1347,6 @@ pub struct File { pub path: Arc, pub mtime: SystemTime, pub(crate) entry_id: Option, - pub(crate) worktree_path: Arc, pub(crate) is_local: bool, } @@ -1367,17 +1359,17 @@ impl language::File for File { &self.path } - fn abs_path(&self) -> Option { + fn abs_path(&self, cx: &AppContext) -> Option { if self.is_local { - Some(self.worktree_path.join(&self.path)) + Some(self.worktree.read(cx).abs_path().join(&self.path)) } else { None } } - fn full_path(&self) -> PathBuf { + fn full_path(&self, cx: &AppContext) -> PathBuf { let mut full_path = PathBuf::new(); - if let Some(worktree_name) = self.worktree_path.file_name() { + if let Some(worktree_name) = self.worktree.read(cx).abs_path().file_name() { full_path.push(worktree_name); } full_path.push(&self.path); @@ -1386,10 +1378,10 @@ impl language::File for File { /// Returns the last component of this handle's absolute path. If this handle refers to the root /// of its worktree, then this method will return the name of the worktree itself. - fn file_name<'a>(&'a self) -> Option { + fn file_name<'a>(&'a self, cx: &AppContext) -> Option { self.path .file_name() - .or_else(|| self.worktree_path.file_name()) + .or_else(|| self.worktree.read(cx).abs_path().file_name()) .map(Into::into) } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 79e41f7704..21df9ec8ce 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -270,6 +270,13 @@ message Worktree { bool weak = 5; } +message File { + uint64 worktree_id = 1; + uint64 entry_id = 2; + string path = 3; + Timestamp mtime = 4; +} + message Entry { uint64 id = 1; bool is_dir = 2; From d192b6ebc7ded22dfcbcdd40439b2191e245d6ff Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 14:44:58 -0700 Subject: [PATCH 2/8] Remove Worktree::abs_path I'd like to only have methods related to absolute paths on local worktrees, because it's not really possible to implement them on remote worktrees since we don't know the full path being shared and wouldn't have anything to do with it anyway if we did. --- crates/editor/src/items.rs | 10 +++------- crates/language/src/buffer.rs | 2 +- crates/project/src/worktree.rs | 27 ++++++++++++--------------- 3 files changed, 16 insertions(+), 23 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 78f5bcc827..a2413f248a 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -122,13 +122,9 @@ impl ItemView for Editor { } fn title(&self, cx: &AppContext) -> String { - let filename = self - .buffer() - .read(cx) - .file(cx) - .and_then(|file| file.file_name(cx)); - if let Some(name) = filename { - name.to_string_lossy().into() + let file = self.buffer().read(cx).file(cx); + if let Some(file) = file { + file.file_name(cx).to_string_lossy().into() } else { "untitled".into() } diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index cceef3992e..3d0ef512d4 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -170,7 +170,7 @@ pub trait File { /// Returns the last component of this handle's absolute path. If this handle refers to the root /// of its worktree, then this method will return the name of the worktree itself. - fn file_name(&self, cx: &AppContext) -> Option; + fn file_name(&self, cx: &AppContext) -> OsString; fn is_deleted(&self) -> bool; diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index ea87a14341..bbdaaeb007 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -543,6 +543,10 @@ impl LocalWorktree { Ok((tree, scan_states_tx)) } + pub fn abs_path(&self) -> &Arc { + &self.abs_path + } + pub fn authorized_logins(&self) -> Vec { self.config.collaborators.clone() } @@ -1092,10 +1096,6 @@ impl Snapshot { } } - pub fn abs_path(&self) -> &Arc { - &self.abs_path - } - pub fn root_entry(&self) -> Option<&Entry> { self.entry_for_path("") } @@ -1360,29 +1360,26 @@ impl language::File for File { } fn abs_path(&self, cx: &AppContext) -> Option { - if self.is_local { - Some(self.worktree.read(cx).abs_path().join(&self.path)) - } else { - None - } + self.worktree + .read(cx) + .as_local() + .map(|local_worktree| local_worktree.abs_path.join(&self.path)) } fn full_path(&self, cx: &AppContext) -> PathBuf { let mut full_path = PathBuf::new(); - if let Some(worktree_name) = self.worktree.read(cx).abs_path().file_name() { - full_path.push(worktree_name); - } + full_path.push(self.worktree.read(cx).root_name()); full_path.push(&self.path); full_path } /// Returns the last component of this handle's absolute path. If this handle refers to the root /// of its worktree, then this method will return the name of the worktree itself. - fn file_name<'a>(&'a self, cx: &AppContext) -> Option { + fn file_name(&self, cx: &AppContext) -> OsString { self.path .file_name() - .or_else(|| self.worktree.read(cx).abs_path().file_name()) - .map(Into::into) + .map(|name| name.into()) + .unwrap_or_else(|| OsString::from(&self.worktree.read(cx).root_name)) } fn is_deleted(&self) -> bool { From 506ce8e03265356f2ff0f817f6eef938b83c815f Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 15:19:14 -0700 Subject: [PATCH 3/8] Introduce LocalSnapshot This allows us to remove the absolute path and scan-related state from the Snapshot. None of this data is relevant or valid on guests. --- crates/project/Cargo.toml | 2 +- crates/project/src/worktree.rs | 143 ++++++++++++++++++--------------- 2 files changed, 81 insertions(+), 64 deletions(-) diff --git a/crates/project/Cargo.toml b/crates/project/Cargo.toml index 6d5e2790ef..d302be874f 100644 --- a/crates/project/Cargo.toml +++ b/crates/project/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "project" version = "0.1.0" -edition = "2018" +edition = "2021" [lib] path = "src/project.rs" diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index bbdaaeb007..3c7e12d1c9 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -30,7 +30,7 @@ use std::{ ffi::{OsStr, OsString}, fmt, future::Future, - ops::Deref, + ops::{Deref, DerefMut}, path::{Path, PathBuf}, sync::{ atomic::{AtomicUsize, Ordering::SeqCst}, @@ -54,9 +54,9 @@ pub enum Worktree { } pub struct LocalWorktree { - snapshot: Snapshot, + snapshot: LocalSnapshot, config: WorktreeConfig, - background_snapshot: Arc>, + background_snapshot: Arc>, last_scan_state_rx: watch::Receiver, _background_scanner_task: Option>, poll_task: Option>, @@ -85,15 +85,34 @@ pub struct RemoteWorktree { #[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, +} + +#[derive(Clone)] +pub struct LocalSnapshot { + abs_path: Arc, + scan_id: usize, + ignores: HashMap, (Arc, usize)>, removed_entry_ids: HashMap, next_entry_id: Arc, + snapshot: Snapshot, +} + +impl Deref for LocalSnapshot { + type Target = Snapshot; + + fn deref(&self) -> &Self::Target { + &self.snapshot + } +} + +impl DerefMut for LocalSnapshot { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.snapshot + } } #[derive(Clone, Debug)] @@ -112,7 +131,7 @@ enum Registration { struct ShareState { project_id: u64, - snapshots_tx: Sender, + snapshots_tx: Sender, _maintain_remote_snapshot: Option>, } @@ -158,7 +177,7 @@ impl Worktree { 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(); + let abs_path = tree.abs_path().clone(); let background_snapshot = tree.background_snapshot.clone(); let background = cx.background().clone(); tree._background_scanner_task = Some(cx.background().spawn(async move { @@ -233,15 +252,10 @@ impl Worktree { cx.add_model(|cx: &mut ModelContext| { let snapshot = Snapshot { id: WorktreeId(remote_id as usize), - scan_id: 0, - abs_path: Path::new("").into(), root_name, root_char_bag, - ignores: Default::default(), entries_by_path, entries_by_id, - removed_entry_ids: Default::default(), - next_entry_id: Default::default(), }; let (updates_tx, mut updates_rx) = postage::mpsc::channel(64); @@ -328,7 +342,7 @@ impl Worktree { pub fn snapshot(&self) -> Snapshot { match self { - Worktree::Local(worktree) => worktree.snapshot(), + Worktree::Local(worktree) => worktree.snapshot().snapshot, Worktree::Remote(worktree) => worktree.snapshot(), } } @@ -469,28 +483,28 @@ impl LocalWorktree { let (scan_states_tx, scan_states_rx) = smol::channel::unbounded(); let (mut last_scan_state_tx, last_scan_state_rx) = watch::channel_with(ScanState::Scanning); let tree = cx.add_model(move |cx: &mut ModelContext| { - let mut snapshot = Snapshot { - id: WorktreeId::from_usize(cx.model_id()), - scan_id: 0, + let mut snapshot = LocalSnapshot { abs_path, - root_name: root_name.clone(), - root_char_bag, + scan_id: 0, ignores: Default::default(), - entries_by_path: Default::default(), - entries_by_id: Default::default(), removed_entry_ids: Default::default(), next_entry_id: Arc::new(next_entry_id), + snapshot: Snapshot { + id: WorktreeId::from_usize(cx.model_id()), + root_name: root_name.clone(), + root_char_bag, + entries_by_path: Default::default(), + entries_by_id: Default::default(), + }, }; if let Some(metadata) = metadata { - snapshot.insert_entry( - Entry::new( - path.into(), - &metadata, - &snapshot.next_entry_id, - snapshot.root_char_bag, - ), - fs.as_ref(), + let entry = Entry::new( + path.into(), + &metadata, + &snapshot.next_entry_id, + snapshot.root_char_bag, ); + snapshot.insert_entry(entry, fs.as_ref()); } let tree = Self { @@ -547,6 +561,18 @@ impl LocalWorktree { &self.abs_path } + 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 authorized_logins(&self) -> Vec { self.config.collaborators.clone() } @@ -628,7 +654,7 @@ impl LocalWorktree { } } - pub fn snapshot(&self) -> Snapshot { + pub fn snapshot(&self) -> LocalSnapshot { self.snapshot.clone() } @@ -754,7 +780,8 @@ impl LocalWorktree { let snapshot = self.snapshot(); let rpc = self.client.clone(); let worktree_id = cx.model_id() as u64; - let (snapshots_to_send_tx, snapshots_to_send_rx) = smol::channel::unbounded::(); + let (snapshots_to_send_tx, snapshots_to_send_rx) = + smol::channel::unbounded::(); let maintain_remote_snapshot = cx.background().spawn({ let rpc = rpc.clone(); let snapshot = snapshot.clone(); @@ -974,9 +1001,6 @@ impl Snapshot { } pub(crate) fn apply_update(&mut self, update: proto::UpdateWorktree) -> Result<()> { - self.scan_id += 1; - let scan_id = self.scan_id; - let mut entries_by_path_edits = Vec::new(); let mut entries_by_id_edits = Vec::new(); for entry_id in update.removed_entries { @@ -997,7 +1021,7 @@ impl Snapshot { id: entry.id, path: entry.path.clone(), is_ignored: entry.is_ignored, - scan_id, + scan_id: 0, })); entries_by_path_edits.push(Edit::Insert(entry)); } @@ -1084,18 +1108,6 @@ 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 root_entry(&self) -> Option<&Entry> { self.entry_for_path("") } @@ -1125,7 +1137,9 @@ impl Snapshot { pub fn inode_for_path(&self, path: impl AsRef) -> Option { self.entry_for_path(path.as_ref()).map(|e| e.inode) } +} +impl LocalSnapshot { fn insert_entry(&mut self, mut entry: Entry, fs: &dyn Fs) -> Entry { if !entry.is_dir() && entry.path.file_name() == Some(&GITIGNORE) { let abs_path = self.abs_path.join(&entry.path); @@ -1147,12 +1161,13 @@ impl Snapshot { self.reuse_entry_id(&mut entry); self.entries_by_path.insert_or_replace(entry.clone(), &()); + let scan_id = self.scan_id; self.entries_by_id.insert_or_replace( PathEntry { id: entry.id, path: entry.path.clone(), is_ignored: entry.is_ignored, - scan_id: self.scan_id, + scan_id, }, &(), ); @@ -1308,7 +1323,7 @@ impl Deref for Worktree { } impl Deref for LocalWorktree { - type Target = Snapshot; + type Target = LocalSnapshot; fn deref(&self) -> &Self::Target { &self.snapshot @@ -1679,14 +1694,14 @@ impl<'a> sum_tree::Dimension<'a, EntrySummary> for PathKey { struct BackgroundScanner { fs: Arc, - snapshot: Arc>, + snapshot: Arc>, notify: Sender, executor: Arc, } impl BackgroundScanner { fn new( - snapshot: Arc>, + snapshot: Arc>, notify: Sender, fs: Arc, executor: Arc, @@ -1703,7 +1718,7 @@ impl BackgroundScanner { self.snapshot.lock().abs_path.clone() } - fn snapshot(&self) -> Snapshot { + fn snapshot(&self) -> LocalSnapshot { self.snapshot.lock().clone() } @@ -2046,7 +2061,7 @@ impl BackgroundScanner { .await; } - async fn update_ignore_status(&self, job: UpdateIgnoreStatusJob, snapshot: &Snapshot) { + async fn update_ignore_status(&self, job: UpdateIgnoreStatusJob, snapshot: &LocalSnapshot) { let mut ignore_stack = job.ignore_stack; if let Some((ignore, _)) = snapshot.ignores.get(&job.path) { ignore_stack = ignore_stack.append(job.path.clone(), ignore.clone()); @@ -2090,7 +2105,7 @@ impl BackgroundScanner { async fn refresh_entry( fs: &dyn Fs, - snapshot: &Mutex, + snapshot: &Mutex, path: Arc, abs_path: &Path, ) -> Result { @@ -2158,7 +2173,7 @@ impl WorktreeHandle for ModelHandle { use smol::future::FutureExt; let filename = "fs-event-sentinel"; - let root_path = cx.read(|cx| self.read(cx).abs_path.clone()); + let root_path = cx.read(|cx| self.read(cx).as_local().unwrap().abs_path().clone()); let tree = self.clone(); async move { std::fs::write(root_path.join(filename), "").unwrap(); @@ -2510,17 +2525,19 @@ mod tests { let (notify_tx, _notify_rx) = smol::channel::unbounded(); let fs = Arc::new(RealFs); let next_entry_id = Arc::new(AtomicUsize::new(0)); - let mut initial_snapshot = Snapshot { - id: WorktreeId::from_usize(0), - scan_id: 0, + let mut initial_snapshot = LocalSnapshot { abs_path: root_dir.path().into(), - entries_by_path: Default::default(), - entries_by_id: Default::default(), + scan_id: 0, removed_entry_ids: Default::default(), ignores: Default::default(), - root_name: Default::default(), - root_char_bag: Default::default(), next_entry_id: next_entry_id.clone(), + snapshot: Snapshot { + id: WorktreeId::from_usize(0), + entries_by_path: Default::default(), + entries_by_id: Default::default(), + root_name: Default::default(), + root_char_bag: Default::default(), + }, }; initial_snapshot.insert_entry( Entry::new( @@ -2756,7 +2773,7 @@ mod tests { .collect() } - impl Snapshot { + impl LocalSnapshot { fn check_invariants(&self) { let mut files = self.files(true, 0); let mut visible_files = self.files(false, 0); From ea9c5b06863cd502ec129d8465f0ac3521d5114d Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 15:21:33 -0700 Subject: [PATCH 4/8] =?UTF-8?q?=F0=9F=92=84?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/project/src/project.rs | 2 +- crates/project/src/worktree.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index b540d9713a..56db25365e 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -2428,7 +2428,7 @@ mod tests { .as_remote_mut() .unwrap() .snapshot - .apply_update(update_message) + .apply_remote_update(update_message) .unwrap(); assert_eq!( diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 3c7e12d1c9..32e72a5f17 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -265,7 +265,7 @@ impl Worktree { .spawn(async move { while let Some(update) = updates_rx.recv().await { let mut snapshot = snapshot_tx.borrow().clone(); - if let Err(error) = snapshot.apply_update(update) { + if let Err(error) = snapshot.apply_remote_update(update) { log::error!("error applying worktree update: {}", error); } *snapshot_tx.borrow_mut() = snapshot; @@ -1000,7 +1000,7 @@ impl Snapshot { } } - pub(crate) fn apply_update(&mut self, update: proto::UpdateWorktree) -> Result<()> { + pub(crate) fn apply_remote_update(&mut self, update: proto::UpdateWorktree) -> Result<()> { let mut entries_by_path_edits = Vec::new(); let mut entries_by_id_edits = Vec::new(); for entry_id in update.removed_entries { @@ -2618,7 +2618,7 @@ mod tests { let update = scanner .snapshot() .build_update(&prev_snapshot, 0, 0, include_ignored); - prev_snapshot.apply_update(update).unwrap(); + prev_snapshot.apply_remote_update(update).unwrap(); assert_eq!( prev_snapshot.to_vec(true), scanner.snapshot().to_vec(include_ignored) From 66fce5ec8e129e574f658db19a1791d1781ec020 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 15:52:14 -0700 Subject: [PATCH 5/8] Introduce LocalFile trait If you want to call `abs_path` or `load`, the file needs to be local. You call `as_local` which returns `Option` with those local-only methods. I think this makes it more explicit what works only locally vs everywhere. --- crates/language/src/buffer.rs | 36 ++++++++++++++++------------ crates/project/src/project.rs | 6 ++--- crates/project/src/worktree.rs | 44 +++++++++++++++++++++------------- 3 files changed, 51 insertions(+), 35 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 3d0ef512d4..9ff432ae86 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -156,14 +156,13 @@ pub enum Event { } pub trait File { + fn as_local(&self) -> Option<&dyn LocalFile>; + fn mtime(&self) -> SystemTime; /// Returns the path of this file relative to the worktree's root directory. fn path(&self) -> &Arc; - /// Returns the absolute path of this file. - fn abs_path(&self, cx: &AppContext) -> Option; - /// Returns the path of this file relative to the worktree's parent directory (this means it /// includes the name of the worktree's root folder). fn full_path(&self, cx: &AppContext) -> PathBuf; @@ -182,8 +181,6 @@ pub trait File { cx: &mut MutableAppContext, ) -> Task>; - fn load_local(&self, cx: &AppContext) -> Option>>; - fn format_remote(&self, buffer_id: u64, cx: &mut MutableAppContext) -> Option>>; @@ -194,6 +191,13 @@ pub trait File { fn as_any(&self) -> &dyn Any; } +pub trait LocalFile: File { + /// Returns the absolute path of this file. + fn abs_path(&self, cx: &AppContext) -> PathBuf; + + fn load(&self, cx: &AppContext) -> Task>; +} + pub(crate) struct QueryCursorHandle(Option); #[derive(Clone)] @@ -457,7 +461,7 @@ impl Buffer { if let Some(LanguageServerState { server, .. }) = self.language_server.as_ref() { let server = server.clone(); - let abs_path = file.abs_path(cx).unwrap(); + let abs_path = file.as_local().unwrap().abs_path(cx); let version = self.version(); cx.spawn(|this, mut cx| async move { let edits = server @@ -634,7 +638,11 @@ impl Buffer { if let Some(new_file) = new_file { self.file = Some(new_file); } - if let Some(state) = &self.language_server { + if let Some((state, local_file)) = &self + .language_server + .as_ref() + .zip(self.file.as_ref().and_then(|f| f.as_local())) + { cx.background() .spawn( state @@ -642,10 +650,7 @@ impl Buffer { .notify::( lsp::DidSaveTextDocumentParams { text_document: lsp::TextDocumentIdentifier { - uri: lsp::Url::from_file_path( - self.file.as_ref().unwrap().abs_path(cx).unwrap(), - ) - .unwrap(), + uri: lsp::Url::from_file_path(local_file.abs_path(cx)).unwrap(), }, text: None, }, @@ -685,7 +690,9 @@ impl Buffer { task = Some(cx.spawn(|this, mut cx| { async move { let new_text = this.read_with(&cx, |this, cx| { - this.file.as_ref().and_then(|file| file.load_local(cx)) + this.file + .as_ref() + .and_then(|file| file.as_local().map(|f| f.load(cx))) }); if let Some(new_text) = new_text { let new_text = new_text.await?; @@ -1235,9 +1242,8 @@ impl Buffer { let abs_path = self .file .as_ref() - .map_or(Path::new("/").to_path_buf(), |file| { - file.abs_path(cx).unwrap() - }); + .and_then(|f| f.as_local()) + .map_or(Path::new("/").to_path_buf(), |file| file.abs_path(cx)); let version = post_inc(&mut language_server.next_version); let snapshot = LanguageServerSnapshot { diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 56db25365e..b78f44f9d5 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -938,7 +938,7 @@ impl Project { let buffer_abs_path; if let Some(file) = File::from_dyn(buffer.file()) { worktree = file.worktree.clone(); - buffer_abs_path = file.abs_path(cx); + buffer_abs_path = file.as_local().map(|f| f.abs_path(cx)); } else { return Task::ready(Err(anyhow!("buffer does not belong to any worktree"))); }; @@ -2181,8 +2181,8 @@ mod tests { cx.update(|cx| { let target_buffer = definition.target_buffer.read(cx); assert_eq!( - target_buffer.file().unwrap().abs_path(cx), - Some(dir.path().join("a.rs")) + target_buffer.file().unwrap().as_local().unwrap().abs_path(cx), + dir.path().join("a.rs") ); assert_eq!(definition.target_range.to_offset(target_buffer), 9..10); assert_eq!( diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 32e72a5f17..4fb359ba1b 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1366,6 +1366,14 @@ pub struct File { } impl language::File for File { + fn as_local(&self) -> Option<&dyn language::LocalFile> { + if self.is_local { + Some(self) + } else { + None + } + } + fn mtime(&self) -> SystemTime { self.mtime } @@ -1374,13 +1382,6 @@ impl language::File for File { &self.path } - fn abs_path(&self, cx: &AppContext) -> Option { - self.worktree - .read(cx) - .as_local() - .map(|local_worktree| local_worktree.abs_path.join(&self.path)) - } - fn full_path(&self, cx: &AppContext) -> PathBuf { let mut full_path = PathBuf::new(); full_path.push(self.worktree.read(cx).root_name()); @@ -1448,16 +1449,6 @@ impl language::File for File { }) } - fn load_local(&self, cx: &AppContext) -> Option>> { - let worktree = self.worktree.read(cx).as_local()?; - let abs_path = worktree.absolutize(&self.path); - let fs = worktree.fs.clone(); - Some( - cx.background() - .spawn(async move { fs.load(&abs_path).await }), - ) - } - fn format_remote( &self, buffer_id: u64, @@ -1510,6 +1501,25 @@ impl language::File for File { } } +impl language::LocalFile for File { + fn abs_path(&self, cx: &AppContext) -> PathBuf { + self.worktree + .read(cx) + .as_local() + .unwrap() + .abs_path + .join(&self.path) + } + + fn load(&self, cx: &AppContext) -> Task> { + let worktree = self.worktree.read(cx).as_local().unwrap(); + let abs_path = worktree.absolutize(&self.path); + let fs = worktree.fs.clone(); + cx.background() + .spawn(async move { fs.load(&abs_path).await }) + } +} + impl File { pub fn from_dyn(file: Option<&dyn language::File>) -> Option<&Self> { file.and_then(|f| f.as_any().downcast_ref()) From da13d028a3963928bad6d574f327ebe5a3ecbc7e Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Sat, 22 Jan 2022 22:19:04 -0700 Subject: [PATCH 6/8] Send File protos as part of Buffer protos Use the File proto to build the File associated with the buffer rather than relying on the local entry. --- crates/language/src/buffer.rs | 3 ++ crates/project/src/worktree.rs | 58 ++++++++++++++++------- crates/rpc/proto/zed.proto | 84 +++++++++++++++++++--------------- crates/rpc/src/proto.rs | 1 + 4 files changed, 92 insertions(+), 54 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 9ff432ae86..1d3b8a8e6d 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -189,6 +189,8 @@ pub trait File { fn buffer_removed(&self, buffer_id: u64, cx: &mut MutableAppContext); fn as_any(&self) -> &dyn Any; + + fn to_proto(&self) -> rpc::proto::File; } pub trait LocalFile: File { @@ -352,6 +354,7 @@ impl Buffer { pub fn to_proto(&self) -> proto::Buffer { proto::Buffer { id: self.remote_id(), + file: self.file.as_ref().map(|f| f.to_proto()), visible_text: self.text.text(), deleted_text: self.text.deleted_text(), undo_map: self diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index 4fb359ba1b..ff3cb1ea82 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -840,11 +840,6 @@ impl RemoteWorktree { let path: Arc = Arc::from(path); let path_string = path.to_string_lossy().to_string(); cx.spawn_weak(move |this, mut cx| async move { - let entry = this - .upgrade(&cx) - .ok_or_else(|| anyhow!("worktree was closed"))? - .read_with(&cx, |tree, _| tree.entry_for_path(&path).cloned()) - .ok_or_else(|| anyhow!("file does not exist"))?; let response = rpc .request(proto::OpenBuffer { project_id, @@ -856,17 +851,15 @@ impl RemoteWorktree { let this = this .upgrade(&cx) .ok_or_else(|| anyhow!("worktree was closed"))?; - let file = File { - entry_id: Some(entry.id), - worktree: this.clone(), - path: entry.path, - mtime: entry.mtime, - is_local: false, - }; - let remote_buffer = response.buffer.ok_or_else(|| anyhow!("empty buffer"))?; - Ok(cx.add_model(|cx| { - Buffer::from_proto(replica_id, remote_buffer, Some(Box::new(file)), cx).unwrap() - })) + let mut remote_buffer = response.buffer.ok_or_else(|| anyhow!("empty buffer"))?; + let file = remote_buffer + .file + .take() + .map(|proto| cx.read(|cx| File::from_proto(proto, this.clone(), cx))) + .transpose()? + .map(|file| Box::new(file) as Box); + + Ok(cx.add_model(|cx| Buffer::from_proto(replica_id, remote_buffer, file, cx).unwrap())) }) } @@ -1499,6 +1492,15 @@ impl language::File for File { fn as_any(&self) -> &dyn Any { self } + + fn to_proto(&self) -> rpc::proto::File { + rpc::proto::File { + worktree_id: self.worktree.id() as u64, + entry_id: self.entry_id.map(|entry_id| entry_id as u64), + path: self.path.to_string_lossy().into(), + mtime: Some(self.mtime.into()), + } + } } impl language::LocalFile for File { @@ -1521,6 +1523,30 @@ impl language::LocalFile for File { } impl File { + pub fn from_proto( + proto: rpc::proto::File, + worktree: ModelHandle, + cx: &AppContext, + ) -> Result { + let worktree_id = worktree + .read(cx) + .as_remote() + .ok_or_else(|| anyhow!("not remote"))? + .id(); + + if worktree_id.to_proto() != proto.worktree_id { + return Err(anyhow!("worktree id does not match file")); + } + + Ok(Self { + worktree, + path: Path::new(&proto.path).into(), + mtime: proto.mtime.ok_or_else(|| anyhow!("no timestamp"))?.into(), + entry_id: proto.entry_id.map(|entry_id| entry_id as usize), + is_local: false, + }) + } + pub fn from_dyn(file: Option<&dyn language::File>) -> Option<&Self> { file.and_then(|f| f.as_any().downcast_ref()) } diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 21df9ec8ce..26fa227dde 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -33,25 +33,26 @@ message Envelope { OpenBufferResponse open_buffer_response = 25; CloseBuffer close_buffer = 26; UpdateBuffer update_buffer = 27; - SaveBuffer save_buffer = 28; - BufferSaved buffer_saved = 29; - FormatBuffer format_buffer = 30; + UpdateBufferFile update_buffer_file = 28; + SaveBuffer save_buffer = 29; + BufferSaved buffer_saved = 30; + FormatBuffer format_buffer = 31; - GetChannels get_channels = 31; - GetChannelsResponse get_channels_response = 32; - JoinChannel join_channel = 33; - JoinChannelResponse join_channel_response = 34; - LeaveChannel leave_channel = 35; - SendChannelMessage send_channel_message = 36; - SendChannelMessageResponse send_channel_message_response = 37; - ChannelMessageSent channel_message_sent = 38; - GetChannelMessages get_channel_messages = 39; - GetChannelMessagesResponse get_channel_messages_response = 40; + GetChannels get_channels = 32; + GetChannelsResponse get_channels_response = 33; + JoinChannel join_channel = 34; + JoinChannelResponse join_channel_response = 35; + LeaveChannel leave_channel = 36; + SendChannelMessage send_channel_message = 37; + SendChannelMessageResponse send_channel_message_response = 38; + ChannelMessageSent channel_message_sent = 39; + GetChannelMessages get_channel_messages = 40; + GetChannelMessagesResponse get_channel_messages_response = 41; - UpdateContacts update_contacts = 41; + UpdateContacts update_contacts = 42; - GetUsers get_users = 42; - GetUsersResponse get_users_response = 43; + GetUsers get_users = 43; + GetUsersResponse get_users_response = 44; } } @@ -88,9 +89,9 @@ message JoinProject { } message JoinProjectResponse { - uint32 replica_id = 2; - repeated Worktree worktrees = 3; - repeated Collaborator collaborators = 4; + uint32 replica_id = 1; + repeated Worktree worktrees = 2; + repeated Collaborator collaborators = 3; } message LeaveProject { @@ -150,7 +151,13 @@ message CloseBuffer { message UpdateBuffer { uint64 project_id = 1; uint64 buffer_id = 2; - repeated Operation operations = 4; + repeated Operation operations = 3; +} + +message UpdateBufferFile { + uint64 project_id = 1; + uint64 buffer_id = 2; + File file = 3; } message SaveBuffer { @@ -177,11 +184,11 @@ message UpdateDiagnosticSummary { } message DiagnosticSummary { - string path = 3; - uint32 error_count = 4; - uint32 warning_count = 5; - uint32 info_count = 6; - uint32 hint_count = 7; + string path = 1; + uint32 error_count = 2; + uint32 warning_count = 3; + uint32 info_count = 4; + uint32 hint_count = 5; } message DiskBasedDiagnosticsUpdating { @@ -272,7 +279,7 @@ message Worktree { message File { uint64 worktree_id = 1; - uint64 entry_id = 2; + optional uint64 entry_id = 2; string path = 3; Timestamp mtime = 4; } @@ -289,15 +296,16 @@ message Entry { message Buffer { uint64 id = 1; - string visible_text = 2; - string deleted_text = 3; - repeated BufferFragment fragments = 4; - repeated UndoMapEntry undo_map = 5; - repeated VectorClockEntry version = 6; - repeated SelectionSet selections = 7; - repeated Diagnostic diagnostics = 8; - uint32 lamport_timestamp = 9; - repeated Operation deferred_operations = 10; + optional File file = 2; + string visible_text = 3; + string deleted_text = 4; + repeated BufferFragment fragments = 5; + repeated UndoMapEntry undo_map = 6; + repeated VectorClockEntry version = 7; + repeated SelectionSet selections = 8; + repeated Diagnostic diagnostics = 9; + uint32 lamport_timestamp = 10; + repeated Operation deferred_operations = 11; } message BufferFragment { @@ -306,7 +314,7 @@ message BufferFragment { uint32 lamport_timestamp = 3; uint32 insertion_offset = 4; uint32 len = 5; - bool visible = 6; + bool visible = 6; repeated VectorClockEntry deletions = 7; repeated VectorClockEntry max_undos = 8; } @@ -390,8 +398,8 @@ message Operation { message UpdateSelections { uint32 replica_id = 1; - uint32 lamport_timestamp = 3; - repeated Selection selections = 4; + uint32 lamport_timestamp = 2; + repeated Selection selections = 3; } } diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 8860bc5f05..0b27ca7e36 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -157,6 +157,7 @@ messages!( UnregisterWorktree, UnshareProject, UpdateBuffer, + UpdateBufferFile, UpdateContacts, UpdateDiagnosticSummary, UpdateWorktree, From 4372fe1ed04ea4c73e558eb487690b8997b13388 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 24 Jan 2022 09:32:40 +0100 Subject: [PATCH 7/8] Maintain remote buffers via `UpdateBufferFile` messages sent by host --- crates/language/src/buffer.rs | 14 +++++--- crates/project/src/project.rs | 65 ++++++++++++++++++++++++++++------- crates/rpc/src/proto.rs | 1 + crates/server/src/rpc.rs | 58 +++++++++++++++++++++++++++---- 4 files changed, 115 insertions(+), 23 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 1d3b8a8e6d..60737b1e43 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -668,10 +668,14 @@ impl Buffer { &mut self, new_file: Box, cx: &mut ModelContext, - ) -> Option> { - let old_file = self.file.as_ref()?; + ) -> Task<()> { + let old_file = if let Some(file) = self.file.as_ref() { + file + } else { + return Task::ready(()); + }; let mut file_changed = false; - let mut task = None; + let mut task = Task::ready(()); if new_file.path() != old_file.path() { file_changed = true; @@ -690,7 +694,7 @@ impl Buffer { file_changed = true; if !self.is_dirty() { - task = Some(cx.spawn(|this, mut cx| { + task = cx.spawn(|this, mut cx| { async move { let new_text = this.read_with(&cx, |this, cx| { this.file @@ -714,7 +718,7 @@ impl Buffer { } .log_err() .map(drop) - })); + }); } } } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index b78f44f9d5..65fdad17c0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -298,6 +298,7 @@ impl Project { Self::handle_disk_based_diagnostics_updated, ), client.subscribe_to_entity(remote_id, cx, Self::handle_update_buffer), + client.subscribe_to_entity(remote_id, cx, Self::handle_update_buffer_file), client.subscribe_to_entity(remote_id, cx, Self::handle_buffer_saved), ], client, @@ -1136,10 +1137,12 @@ 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(); + if worktree.read(cx).is_local() { + cx.subscribe(&worktree, |this, worktree, _, cx| { + this.update_local_worktree_buffers(worktree, cx); + }) + .detach(); + } let push_weak_handle = { let worktree = worktree.read(cx); @@ -1161,12 +1164,11 @@ impl Project { cx.notify(); } - fn update_open_buffers( + fn update_local_worktree_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 mut buffers_to_delete = Vec::new(); for (buffer_id, buffer) in &self.open_buffers { @@ -1183,7 +1185,7 @@ impl Project { .and_then(|entry_id| snapshot.entry_for_id(entry_id)) { File { - is_local: local, + is_local: true, entry_id: Some(entry.id), mtime: entry.mtime, path: entry.path.clone(), @@ -1193,7 +1195,7 @@ impl Project { snapshot.entry_for_path(old_file.path().as_ref()) { File { - is_local: local, + is_local: true, entry_id: Some(entry.id), mtime: entry.mtime, path: entry.path.clone(), @@ -1201,7 +1203,7 @@ impl Project { } } else { File { - is_local: local, + is_local: true, entry_id: None, path: old_file.path().clone(), mtime: old_file.mtime(), @@ -1209,9 +1211,18 @@ impl Project { } }; - if let Some(task) = buffer.file_updated(Box::new(new_file), cx) { - task.detach(); + if let Some(project_id) = self.remote_id() { + let client = self.client.clone(); + let message = proto::UpdateBufferFile { + project_id, + buffer_id: *buffer_id as u64, + file: Some(new_file.to_proto()), + }; + cx.foreground() + .spawn(async move { client.send(message).await }) + .detach_and_log_err(cx); } + buffer.file_updated(Box::new(new_file), cx).detach(); } }); } else { @@ -1492,6 +1503,31 @@ impl Project { Ok(()) } + pub fn handle_update_buffer_file( + &mut self, + envelope: TypedEnvelope, + _: Arc, + cx: &mut ModelContext, + ) -> Result<()> { + let payload = envelope.payload.clone(); + let buffer_id = payload.buffer_id as usize; + let file = payload.file.ok_or_else(|| anyhow!("invalid file"))?; + let worktree = self + .worktree_for_id(WorktreeId::from_proto(file.worktree_id), cx) + .ok_or_else(|| anyhow!("no such worktree"))?; + let file = File::from_proto(file, worktree.clone(), cx)?; + let buffer = self + .open_buffers + .get_mut(&buffer_id) + .and_then(|b| b.upgrade(cx)) + .ok_or_else(|| anyhow!("no such buffer"))?; + buffer.update(cx, |buffer, cx| { + buffer.file_updated(Box::new(file), cx).detach(); + }); + + Ok(()) + } + pub fn handle_save_buffer( &mut self, envelope: TypedEnvelope, @@ -2181,7 +2217,12 @@ mod tests { cx.update(|cx| { let target_buffer = definition.target_buffer.read(cx); assert_eq!( - target_buffer.file().unwrap().as_local().unwrap().abs_path(cx), + target_buffer + .file() + .unwrap() + .as_local() + .unwrap() + .abs_path(cx), dir.path().join("a.rs") ); assert_eq!(definition.target_range.to_offset(target_buffer), 9..10); diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 0b27ca7e36..578179667b 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -198,6 +198,7 @@ entity_messages!( UnregisterWorktree, UnshareProject, UpdateBuffer, + UpdateBufferFile, UpdateDiagnosticSummary, UpdateWorktree, ); diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index 4ad2382699..fbb5c4adc5 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -77,6 +77,7 @@ impl Server { .add_handler(Server::open_buffer) .add_handler(Server::close_buffer) .add_handler(Server::update_buffer) + .add_handler(Server::update_buffer_file) .add_handler(Server::buffer_saved) .add_handler(Server::save_buffer) .add_handler(Server::format_buffer) @@ -704,6 +705,22 @@ impl Server { Ok(()) } + async fn update_buffer_file( + self: Arc, + request: TypedEnvelope, + ) -> tide::Result<()> { + let receiver_ids = self + .state() + .project_connection_ids(request.payload.project_id, request.sender_id) + .ok_or_else(|| anyhow!(NO_SUCH_PROJECT))?; + broadcast(request.sender_id, receiver_ids, |connection_id| { + self.peer + .forward_send(request.sender_id, connection_id, request.payload.clone()) + }) + .await?; + Ok(()) + } + async fn buffer_saved( self: Arc, request: TypedEnvelope, @@ -1470,9 +1487,7 @@ 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, _| { - dbg!(buf.text()) == "i-am-c, i-am-b, i-am-a" - }) + .condition(&mut cx_b, |buf, _| 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") @@ -1490,7 +1505,10 @@ mod tests { buffer_b.read_with(&cx_b, |buf, _| assert!(!buf.is_dirty())); buffer_c.condition(&cx_c, |buf, _| !buf.is_dirty()).await; - // Make changes on host's file system, see those changes on the guests. + // Make changes on host's file system, see those changes on guest worktrees. + fs.rename("/a/file1".as_ref(), "/a/file1-renamed".as_ref()) + .await + .unwrap(); fs.rename("/a/file2".as_ref(), "/a/file3".as_ref()) .await .unwrap(); @@ -1498,18 +1516,29 @@ mod tests { .await .unwrap(); + worktree_a + .condition(&cx_a, |tree, _| tree.file_count() == 4) + .await; worktree_b .condition(&cx_b, |tree, _| tree.file_count() == 4) .await; worktree_c .condition(&cx_c, |tree, _| tree.file_count() == 4) .await; + worktree_a.read_with(&cx_a, |tree, _| { + assert_eq!( + tree.paths() + .map(|p| p.to_string_lossy()) + .collect::>(), + &[".zed.toml", "file1-renamed", "file3", "file4"] + ) + }); worktree_b.read_with(&cx_b, |tree, _| { assert_eq!( tree.paths() .map(|p| p.to_string_lossy()) .collect::>(), - &[".zed.toml", "file1", "file3", "file4"] + &[".zed.toml", "file1-renamed", "file3", "file4"] ) }); worktree_c.read_with(&cx_c, |tree, _| { @@ -1517,9 +1546,26 @@ mod tests { tree.paths() .map(|p| p.to_string_lossy()) .collect::>(), - &[".zed.toml", "file1", "file3", "file4"] + &[".zed.toml", "file1-renamed", "file3", "file4"] ) }); + + // Ensure buffer files are updated as well. + buffer_a + .condition(&cx_a, |buf, _| { + buf.file().unwrap().path().to_str() == Some("file1-renamed") + }) + .await; + buffer_b + .condition(&cx_b, |buf, _| { + buf.file().unwrap().path().to_str() == Some("file1-renamed") + }) + .await; + buffer_c + .condition(&cx_c, |buf, _| { + buf.file().unwrap().path().to_str() == Some("file1-renamed") + }) + .await; } #[gpui::test] From f859d444ff10f29e9d6c1faab3d3ce6d369353a5 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 24 Jan 2022 10:17:36 +0100 Subject: [PATCH 8/8] Don't show conflict indicator on remote buffer after a reload --- crates/language/src/buffer.rs | 27 ++++++++-- crates/project/src/project.rs | 32 +++++++++++ crates/project/src/worktree.rs | 22 ++++++++ crates/rpc/proto/zed.proto | 36 ++++++++----- crates/rpc/src/proto.rs | 2 + crates/server/src/rpc.rs | 99 ++++++++++++++++++++++++++++++++++ 6 files changed, 201 insertions(+), 17 deletions(-) diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 60737b1e43..bd361e9731 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -198,6 +198,14 @@ pub trait LocalFile: File { fn abs_path(&self, cx: &AppContext) -> PathBuf; fn load(&self, cx: &AppContext) -> Task>; + + fn buffer_reloaded( + &self, + buffer_id: u64, + version: &clock::Global, + mtime: SystemTime, + cx: &mut MutableAppContext, + ); } pub(crate) struct QueryCursorHandle(Option); @@ -664,6 +672,21 @@ impl Buffer { cx.emit(Event::Saved); } + pub fn did_reload( + &mut self, + version: clock::Global, + mtime: SystemTime, + cx: &mut ModelContext, + ) { + self.saved_mtime = mtime; + self.saved_version = version; + if let Some(file) = self.file.as_ref().and_then(|f| f.as_local()) { + file.buffer_reloaded(self.remote_id(), &self.saved_version, self.saved_mtime, cx); + } + cx.emit(Event::Reloaded); + cx.notify(); + } + pub fn file_updated( &mut self, new_file: Box, @@ -708,9 +731,7 @@ impl Buffer { .await; this.update(&mut cx, |this, cx| { if this.apply_diff(diff, cx) { - this.saved_version = this.version(); - this.saved_mtime = new_mtime; - cx.emit(Event::Reloaded); + this.did_reload(this.version(), new_mtime, cx); } }); } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 65fdad17c0..04f8aa511a 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -299,6 +299,7 @@ impl Project { ), client.subscribe_to_entity(remote_id, cx, Self::handle_update_buffer), client.subscribe_to_entity(remote_id, cx, Self::handle_update_buffer_file), + client.subscribe_to_entity(remote_id, cx, Self::handle_buffer_reloaded), client.subscribe_to_entity(remote_id, cx, Self::handle_buffer_saved), ], client, @@ -1694,6 +1695,37 @@ impl Project { Ok(()) } + pub fn handle_buffer_reloaded( + &mut self, + envelope: TypedEnvelope, + _: Arc, + cx: &mut ModelContext, + ) -> Result<()> { + 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_reload(version, mtime, cx); + Result::<_, anyhow::Error>::Ok(()) + })?; + } + Ok(()) + } + pub fn match_paths<'a>( &self, query: &'a str, diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index ff3cb1ea82..fd33386ffc 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1520,6 +1520,28 @@ impl language::LocalFile for File { cx.background() .spawn(async move { fs.load(&abs_path).await }) } + + fn buffer_reloaded( + &self, + buffer_id: u64, + version: &clock::Global, + mtime: SystemTime, + cx: &mut MutableAppContext, + ) { + let worktree = self.worktree.read(cx).as_local().unwrap(); + if let Some(project_id) = worktree.share.as_ref().map(|share| share.project_id) { + let rpc = worktree.client.clone(); + let message = proto::BufferReloaded { + project_id, + buffer_id, + version: version.into(), + mtime: Some(mtime.into()), + }; + cx.background() + .spawn(async move { rpc.send(message).await }) + .detach_and_log_err(cx); + } + } } impl File { diff --git a/crates/rpc/proto/zed.proto b/crates/rpc/proto/zed.proto index 26fa227dde..cbe7ca5d1a 100644 --- a/crates/rpc/proto/zed.proto +++ b/crates/rpc/proto/zed.proto @@ -36,23 +36,24 @@ message Envelope { UpdateBufferFile update_buffer_file = 28; SaveBuffer save_buffer = 29; BufferSaved buffer_saved = 30; - FormatBuffer format_buffer = 31; + BufferReloaded buffer_reloaded = 31; + FormatBuffer format_buffer = 32; - GetChannels get_channels = 32; - GetChannelsResponse get_channels_response = 33; - JoinChannel join_channel = 34; - JoinChannelResponse join_channel_response = 35; - LeaveChannel leave_channel = 36; - SendChannelMessage send_channel_message = 37; - SendChannelMessageResponse send_channel_message_response = 38; - ChannelMessageSent channel_message_sent = 39; - GetChannelMessages get_channel_messages = 40; - GetChannelMessagesResponse get_channel_messages_response = 41; + GetChannels get_channels = 33; + GetChannelsResponse get_channels_response = 34; + JoinChannel join_channel = 35; + JoinChannelResponse join_channel_response = 36; + LeaveChannel leave_channel = 37; + SendChannelMessage send_channel_message = 38; + SendChannelMessageResponse send_channel_message_response = 39; + ChannelMessageSent channel_message_sent = 40; + GetChannelMessages get_channel_messages = 41; + GetChannelMessagesResponse get_channel_messages_response = 42; - UpdateContacts update_contacts = 42; + UpdateContacts update_contacts = 43; - GetUsers get_users = 43; - GetUsersResponse get_users_response = 44; + GetUsers get_users = 44; + GetUsersResponse get_users_response = 45; } } @@ -172,6 +173,13 @@ message BufferSaved { Timestamp mtime = 4; } +message BufferReloaded { + uint64 project_id = 1; + uint64 buffer_id = 2; + repeated VectorClockEntry version = 3; + Timestamp mtime = 4; +} + message FormatBuffer { uint64 project_id = 1; uint64 buffer_id = 2; diff --git a/crates/rpc/src/proto.rs b/crates/rpc/src/proto.rs index 578179667b..cebe3504e9 100644 --- a/crates/rpc/src/proto.rs +++ b/crates/rpc/src/proto.rs @@ -122,6 +122,7 @@ macro_rules! entity_messages { messages!( Ack, AddProjectCollaborator, + BufferReloaded, BufferSaved, ChannelMessageSent, CloseBuffer, @@ -184,6 +185,7 @@ request_messages!( entity_messages!( project_id, AddProjectCollaborator, + BufferReloaded, BufferSaved, CloseBuffer, DiskBasedDiagnosticsUpdated, diff --git a/crates/server/src/rpc.rs b/crates/server/src/rpc.rs index fbb5c4adc5..c8af5ce1fd 100644 --- a/crates/server/src/rpc.rs +++ b/crates/server/src/rpc.rs @@ -78,6 +78,7 @@ impl Server { .add_handler(Server::close_buffer) .add_handler(Server::update_buffer) .add_handler(Server::update_buffer_file) + .add_handler(Server::buffer_reloaded) .add_handler(Server::buffer_saved) .add_handler(Server::save_buffer) .add_handler(Server::format_buffer) @@ -721,6 +722,22 @@ impl Server { Ok(()) } + async fn buffer_reloaded( + self: Arc, + request: TypedEnvelope, + ) -> tide::Result<()> { + let receiver_ids = self + .state() + .project_connection_ids(request.payload.project_id, request.sender_id) + .ok_or_else(|| anyhow!(NO_SUCH_PROJECT))?; + broadcast(request.sender_id, receiver_ids, |connection_id| { + self.peer + .forward_send(request.sender_id, connection_id, request.payload.clone()) + }) + .await?; + Ok(()) + } + async fn buffer_saved( self: Arc, request: TypedEnvelope, @@ -1661,6 +1678,88 @@ mod tests { }); } + #[gpui::test] + async fn test_buffer_reloading(mut cx_a: TestAppContext, mut cx_b: TestAppContext) { + cx_a.foreground().forbid_parking(); + let lang_registry = Arc::new(LanguageRegistry::new()); + let fs = Arc::new(FakeFs::new()); + + // Connect to a server as 2 clients. + let mut server = TestServer::start(cx_a.foreground()).await; + let client_a = server.create_client(&mut cx_a, "user_a").await; + let client_b = server.create_client(&mut cx_b, "user_b").await; + + // Share a project as client A + fs.insert_tree( + "/dir", + json!({ + ".zed.toml": r#"collaborators = ["user_b", "user_c"]"#, + "a.txt": "a-contents", + }), + ) + .await; + + let project_a = cx_a.update(|cx| { + Project::local( + client_a.clone(), + client_a.user_store.clone(), + lang_registry.clone(), + fs.clone(), + 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 + .read_with(&cx_a, |tree, _| tree.as_local().unwrap().scan_complete()) + .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, |p, cx| p.share(cx)) + .await + .unwrap(); + + // Join that project as client B + let project_b = Project::remote( + project_id, + client_b.clone(), + client_b.user_store.clone(), + lang_registry.clone(), + fs.clone(), + &mut cx_b.to_async(), + ) + .await + .unwrap(); + 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 = project_b + .update(&mut cx_b, |p, cx| p.open_buffer((worktree_id, "a.txt"), cx)) + .await + .unwrap(); + buffer_b.read_with(&cx_b, |buf, _| { + assert!(!buf.is_dirty()); + assert!(!buf.has_conflict()); + }); + + fs.save(Path::new("/dir/a.txt"), &"new contents".into()) + .await + .unwrap(); + buffer_b + .condition(&cx_b, |buf, _| { + buf.text() == "new contents" && !buf.is_dirty() + }) + .await; + buffer_b.read_with(&cx_b, |buf, _| { + assert!(!buf.has_conflict()); + }); + } + #[gpui::test] async fn test_editing_while_guest_opens_buffer( mut cx_a: TestAppContext,