From 8e70e1934ab9bfff1621e5bdd9cb13ed3ff4896a Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 10 Nov 2022 09:33:57 -0700 Subject: [PATCH 1/2] Avoid unwrapping when computing tab description A bug caused the assumptions of this method to be violated. We will fix that in the next commit, but we want to be more conservative in our assumptions here going forward. Co-Authored-By: Antonio Scandurra --- crates/editor/src/items.rs | 86 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 82 insertions(+), 4 deletions(-) diff --git a/crates/editor/src/items.rs b/crates/editor/src/items.rs index 47e06fc545..a80a6b5298 100644 --- a/crates/editor/src/items.rs +++ b/crates/editor/src/items.rs @@ -819,11 +819,20 @@ impl StatusItemView for CursorPosition { fn path_for_buffer<'a>( buffer: &ModelHandle, - mut height: usize, + height: usize, include_filename: bool, cx: &'a AppContext, ) -> Option> { let file = buffer.read(cx).as_singleton()?.read(cx).file()?; + path_for_file(file, height, include_filename, cx) +} + +fn path_for_file<'a>( + file: &'a dyn language::File, + mut height: usize, + include_filename: bool, + cx: &'a AppContext, +) -> Option> { // Ensure we always render at least the filename. height += 1; @@ -845,13 +854,82 @@ fn path_for_buffer<'a>( if include_filename { Some(full_path.into()) } else { - Some(full_path.parent().unwrap().to_path_buf().into()) + Some(full_path.parent()?.to_path_buf().into()) } } else { - let mut path = file.path().strip_prefix(prefix).unwrap(); + let mut path = file.path().strip_prefix(prefix).ok()?; if !include_filename { - path = path.parent().unwrap(); + path = path.parent()?; } Some(path.into()) } } + +#[cfg(test)] +mod tests { + use super::*; + use gpui::MutableAppContext; + use std::{ + path::{Path, PathBuf}, + sync::Arc, + }; + + #[gpui::test] + fn test_path_for_file(cx: &mut MutableAppContext) { + let file = TestFile { + path: Path::new("").into(), + full_path: PathBuf::from(""), + }; + assert_eq!(path_for_file(&file, 0, false, cx), None); + } + + struct TestFile { + path: Arc, + full_path: PathBuf, + } + + impl language::File for TestFile { + fn path(&self) -> &Arc { + &self.path + } + + fn full_path(&self, _: &gpui::AppContext) -> PathBuf { + self.full_path.clone() + } + + fn as_local(&self) -> Option<&dyn language::LocalFile> { + todo!() + } + + fn mtime(&self) -> std::time::SystemTime { + todo!() + } + + fn file_name<'a>(&'a self, _: &'a gpui::AppContext) -> &'a std::ffi::OsStr { + todo!() + } + + fn is_deleted(&self) -> bool { + todo!() + } + + fn save( + &self, + _: u64, + _: language::Rope, + _: clock::Global, + _: project::LineEnding, + _: &mut MutableAppContext, + ) -> gpui::Task> { + todo!() + } + + fn as_any(&self) -> &dyn std::any::Any { + todo!() + } + + fn to_proto(&self) -> rpc::proto::File { + todo!() + } + } +} From fb03eb7a3c8f2b365033d178f354287c580ce684 Mon Sep 17 00:00:00 2001 From: Nathan Sobo Date: Thu, 10 Nov 2022 09:34:16 -0700 Subject: [PATCH 2/2] Store absolute path on server when sharing worktree Co-Authored-By: Antonio Scandurra --- crates/collab/src/integration_tests.rs | 7 +++++++ crates/collab/src/rpc.rs | 6 +++--- crates/collab/src/rpc/store.rs | 16 +++++++++++++--- crates/project/src/worktree.rs | 8 ++++---- 4 files changed, 27 insertions(+), 10 deletions(-) diff --git a/crates/collab/src/integration_tests.rs b/crates/collab/src/integration_tests.rs index 2bf2701f23..d3bf1f28e5 100644 --- a/crates/collab/src/integration_tests.rs +++ b/crates/collab/src/integration_tests.rs @@ -5987,6 +5987,13 @@ async fn test_random_collaboration( guest_client.username, id ); + assert_eq!( + guest_snapshot.abs_path(), + host_snapshot.abs_path(), + "{} has different abs path than the host for worktree {}", + guest_client.username, + id + ); assert_eq!( guest_snapshot.entries(false).collect::>(), host_snapshot.entries(false).collect::>(), diff --git a/crates/collab/src/rpc.rs b/crates/collab/src/rpc.rs index 9fd9bef825..3a92c3ef14 100644 --- a/crates/collab/src/rpc.rs +++ b/crates/collab/src/rpc.rs @@ -42,7 +42,6 @@ use std::{ marker::PhantomData, net::SocketAddr, ops::{Deref, DerefMut}, - os::unix::prelude::OsStrExt, rc::Rc, sync::{ atomic::{AtomicBool, Ordering::SeqCst}, @@ -1024,7 +1023,7 @@ impl Server { id: *id, root_name: worktree.root_name.clone(), visible: worktree.visible, - abs_path: worktree.abs_path.as_os_str().as_bytes().to_vec(), + abs_path: worktree.abs_path.clone(), }) .collect::>(); @@ -1075,7 +1074,7 @@ impl Server { let message = proto::UpdateWorktree { project_id: project_id.to_proto(), worktree_id: *worktree_id, - abs_path: worktree.abs_path.as_os_str().as_bytes().to_vec(), + abs_path: worktree.abs_path.clone(), root_name: worktree.root_name.clone(), updated_entries: worktree.entries.values().cloned().collect(), removed_entries: Default::default(), @@ -1195,6 +1194,7 @@ impl Server { project_id, worktree_id, &request.payload.root_name, + &request.payload.abs_path, &request.payload.removed_entries, &request.payload.updated_entries, request.payload.scan_id, diff --git a/crates/collab/src/rpc/store.rs b/crates/collab/src/rpc/store.rs index c9358ddc2a..8306978c9c 100644 --- a/crates/collab/src/rpc/store.rs +++ b/crates/collab/src/rpc/store.rs @@ -67,7 +67,7 @@ pub struct Collaborator { #[derive(Default, Serialize)] pub struct Worktree { - pub abs_path: PathBuf, + pub abs_path: Vec, pub root_name: String, pub visible: bool, #[serde(skip)] @@ -773,7 +773,11 @@ impl Store { Worktree { root_name: worktree.root_name, visible: worktree.visible, - ..Default::default() + abs_path: worktree.abs_path.clone(), + entries: Default::default(), + diagnostic_summaries: Default::default(), + scan_id: Default::default(), + is_complete: Default::default(), }, ) }) @@ -852,7 +856,11 @@ impl Store { Worktree { root_name: worktree.root_name.clone(), visible: worktree.visible, - ..Default::default() + abs_path: worktree.abs_path.clone(), + entries: Default::default(), + diagnostic_summaries: Default::default(), + scan_id: Default::default(), + is_complete: false, }, ); } @@ -1006,6 +1014,7 @@ impl Store { project_id: ProjectId, worktree_id: u64, worktree_root_name: &str, + worktree_abs_path: &[u8], removed_entries: &[u64], updated_entries: &[proto::Entry], scan_id: u64, @@ -1016,6 +1025,7 @@ impl Store { let connection_ids = project.connection_ids(); let mut worktree = project.worktrees.entry(worktree_id).or_default(); worktree.root_name = worktree_root_name.to_string(); + worktree.abs_path = worktree_abs_path.to_vec(); for entry_id in removed_entries { worktree.entries.remove(entry_id); diff --git a/crates/project/src/worktree.rs b/crates/project/src/worktree.rs index db8fb8e3ff..055aa06706 100644 --- a/crates/project/src/worktree.rs +++ b/crates/project/src/worktree.rs @@ -1179,6 +1179,10 @@ impl Snapshot { self.id } + pub fn abs_path(&self) -> &Arc { + &self.abs_path + } + pub fn contains_entry(&self, entry_id: ProjectEntryId) -> bool { self.entries_by_id.get(&entry_id, &()).is_some() } @@ -1370,10 +1374,6 @@ impl Snapshot { } impl LocalSnapshot { - pub fn abs_path(&self) -> &Arc { - &self.abs_path - } - pub fn extension_counts(&self) -> &HashMap { &self.extension_counts }