Introduce a WorktreeId struct, fix incorrect use of remote worktrees' handle id

Max Brunsfeld created

Change summary

crates/file_finder/src/file_finder.rs     |  6 
crates/project/src/project.rs             | 42 ++++++++-----
crates/project/src/worktree.rs            | 78 ++++++++++++++++--------
crates/project_panel/src/project_panel.rs | 46 ++++++++------
crates/workspace/src/workspace.rs         |  6 
crates/zed/src/zed.rs                     |  2 
6 files changed, 112 insertions(+), 68 deletions(-)

Detailed changes

crates/file_finder/src/file_finder.rs 🔗

@@ -12,7 +12,7 @@ use gpui::{
     ViewContext, ViewHandle, WeakViewHandle,
 };
 use postage::watch;
-use project::{Project, ProjectPath};
+use project::{Project, ProjectPath, WorktreeId};
 use std::{
     cmp,
     path::Path,
@@ -195,7 +195,7 @@ impl FileFinder {
         .with_style(style.container);
 
         let action = Select(ProjectPath {
-            worktree_id: path_match.worktree_id,
+            worktree_id: WorktreeId::from_usize(path_match.worktree_id),
             path: path_match.path.clone(),
         });
         EventHandler::new(container.boxed())
@@ -370,7 +370,7 @@ impl FileFinder {
     fn confirm(&mut self, _: &Confirm, cx: &mut ViewContext<Self>) {
         if let Some(m) = self.matches.get(self.selected_index()) {
             cx.emit(Event::Selected(ProjectPath {
-                worktree_id: m.worktree_id,
+                worktree_id: WorktreeId::from_usize(m.worktree_id),
                 path: m.path.clone(),
             }));
         }

crates/project/src/project.rs 🔗

@@ -59,13 +59,13 @@ pub struct Collaborator {
 #[derive(Debug)]
 pub enum Event {
     ActiveEntryChanged(Option<ProjectEntry>),
-    WorktreeRemoved(usize),
+    WorktreeRemoved(WorktreeId),
     DiagnosticsUpdated(ProjectPath),
 }
 
 #[derive(Clone, Debug, Eq, PartialEq, Hash)]
 pub struct ProjectPath {
-    pub worktree_id: usize,
+    pub worktree_id: WorktreeId,
     pub path: Arc<Path>,
 }
 
@@ -104,7 +104,7 @@ impl DiagnosticSummary {
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
 pub struct ProjectEntry {
-    pub worktree_id: usize,
+    pub worktree_id: WorktreeId,
     pub entry_id: usize,
 }
 
@@ -320,7 +320,11 @@ impl Project {
         &self.worktrees
     }
 
-    pub fn worktree_for_id(&self, id: usize, cx: &AppContext) -> Option<ModelHandle<Worktree>> {
+    pub fn worktree_for_id(
+        &self,
+        id: WorktreeId,
+        cx: &AppContext,
+    ) -> Option<ModelHandle<Worktree>> {
         self.worktrees
             .iter()
             .find(|worktree| worktree.read(cx).id() == id)
@@ -478,7 +482,7 @@ impl Project {
         cx.subscribe(&worktree, |_, worktree, event, cx| match event {
             worktree::Event::DiagnosticsUpdated(path) => {
                 cx.emit(Event::DiagnosticsUpdated(ProjectPath {
-                    worktree_id: worktree.id(),
+                    worktree_id: worktree.read(cx).id(),
                     path: path.clone(),
                 }));
             }
@@ -538,9 +542,9 @@ impl Project {
         cx: &'a AppContext,
     ) -> impl Iterator<Item = (ProjectPath, DiagnosticSummary)> + 'a {
         self.worktrees.iter().flat_map(move |worktree| {
+            let worktree = worktree.read(cx);
             let worktree_id = worktree.id();
             worktree
-                .read(cx)
                 .diagnostic_summaries()
                 .map(move |(path, summary)| (ProjectPath { worktree_id, path }, summary))
         })
@@ -658,9 +662,9 @@ impl Project {
         _: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
-        self.worktrees.retain(|worktree| {
-            worktree.read(cx).as_remote().unwrap().remote_id() != envelope.payload.worktree_id
-        });
+        let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id);
+        self.worktrees
+            .retain(|worktree| worktree.read(cx).as_remote().unwrap().id() != worktree_id);
         cx.notify();
         Ok(())
     }
@@ -671,7 +675,8 @@ impl Project {
         _: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
-        if let Some(worktree) = self.worktree_for_id(envelope.payload.worktree_id as usize, cx) {
+        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| {
                 let worktree = worktree.as_remote_mut().unwrap();
                 worktree.update_from_remote(envelope, cx)
@@ -686,7 +691,8 @@ impl Project {
         _: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
-        if let Some(worktree) = self.worktree_for_id(envelope.payload.worktree_id as usize, cx) {
+        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)
             })?;
@@ -700,7 +706,8 @@ impl Project {
         rpc: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
-        if let Some(worktree) = self.worktree_for_id(envelope.payload.worktree_id as usize, cx) {
+        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)
             })?;
@@ -714,7 +721,8 @@ impl Project {
         rpc: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> anyhow::Result<()> {
-        if let Some(worktree) = self.worktree_for_id(envelope.payload.worktree_id as usize, cx) {
+        let worktree_id = WorktreeId::from_proto(envelope.payload.worktree_id);
+        if let Some(worktree) = self.worktree_for_id(worktree_id, cx) {
             return worktree.update(cx, |worktree, cx| {
                 worktree.handle_open_buffer(envelope, rpc, cx)
             });
@@ -729,7 +737,8 @@ impl Project {
         rpc: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> anyhow::Result<()> {
-        if let Some(worktree) = self.worktree_for_id(envelope.payload.worktree_id as usize, cx) {
+        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_close_buffer(envelope, rpc, cx)
             })?;
@@ -743,7 +752,8 @@ impl Project {
         _: Arc<Client>,
         cx: &mut ModelContext<Self>,
     ) -> Result<()> {
-        if let Some(worktree) = self.worktree_for_id(envelope.payload.worktree_id as usize, cx) {
+        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)
             })?;
@@ -796,7 +806,7 @@ impl<'a> PathMatchCandidateSet<'a> for CandidateSet {
     type Candidates = CandidateSetIter<'a>;
 
     fn id(&self) -> usize {
-        self.snapshot.id()
+        self.snapshot.id().to_usize()
     }
 
     fn len(&self) -> usize {

crates/project/src/worktree.rs 🔗

@@ -61,6 +61,9 @@ enum ScanState {
     Err(Arc<anyhow::Error>),
 }
 
+#[derive(Copy, Clone, PartialEq, Eq, Debug, Hash)]
+pub struct WorktreeId(usize);
+
 pub enum Worktree {
     Local(LocalWorktree),
     Remote(RemoteWorktree),
@@ -171,7 +174,7 @@ impl Worktree {
         let worktree = cx.update(|cx| {
             cx.add_model(|cx: &mut ModelContext<Worktree>| {
                 let snapshot = Snapshot {
-                    id: remote_id as usize,
+                    id: WorktreeId(remote_id as usize),
                     scan_id: 0,
                     abs_path: Path::new("").into(),
                     root_name,
@@ -214,7 +217,6 @@ impl Worktree {
 
                 Worktree::Remote(RemoteWorktree {
                     project_id: project_remote_id,
-                    remote_id,
                     replica_id,
                     snapshot,
                     snapshot_rx,
@@ -824,16 +826,13 @@ impl Worktree {
         cx: &mut ModelContext<Self>,
     ) {
         if let Some((project_id, worktree_id, rpc)) = match self {
-            Worktree::Local(worktree) => worktree.share.as_ref().map(|share| {
-                (
-                    share.project_id,
-                    worktree.id() as u64,
-                    worktree.client.clone(),
-                )
-            }),
+            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.remote_id,
+                worktree.snapshot.id(),
                 worktree.client.clone(),
             )),
         } {
@@ -841,7 +840,7 @@ impl Worktree {
                 if let Err(error) = rpc
                     .request(proto::UpdateBuffer {
                         project_id,
-                        worktree_id,
+                        worktree_id: worktree_id.0 as u64,
                         buffer_id,
                         operations: vec![language::proto::serialize_operation(&operation)],
                     })
@@ -862,9 +861,33 @@ 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: usize,
+    id: WorktreeId,
     scan_id: usize,
     abs_path: Arc<Path>,
     root_name: String,
@@ -906,7 +929,6 @@ struct ShareState {
 
 pub struct RemoteWorktree {
     project_id: u64,
-    remote_id: u64,
     snapshot: Snapshot,
     snapshot_rx: watch::Receiver<Snapshot>,
     client: Arc<Client>,
@@ -962,7 +984,7 @@ impl LocalWorktree {
         let (mut last_scan_state_tx, last_scan_state_rx) = watch::channel_with(ScanState::Scanning);
         let tree = cx.add_model(move |cx: &mut ModelContext<Worktree>| {
             let mut snapshot = Snapshot {
-                id: cx.model_id(),
+                id: WorktreeId::from_usize(cx.model_id()),
                 scan_id: 0,
                 abs_path,
                 root_name: root_name.clone(),
@@ -1426,6 +1448,14 @@ impl Deref for LocalWorktree {
     }
 }
 
+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)
@@ -1433,10 +1463,6 @@ impl fmt::Debug for LocalWorktree {
 }
 
 impl RemoteWorktree {
-    pub fn remote_id(&self) -> u64 {
-        self.remote_id
-    }
-
     fn get_open_buffer(
         &mut self,
         path: &Path,
@@ -1467,7 +1493,7 @@ impl RemoteWorktree {
         let rpc = self.client.clone();
         let replica_id = self.replica_id;
         let project_id = self.project_id;
-        let remote_worktree_id = self.remote_id;
+        let remote_worktree_id = self.id();
         let root_path = self.snapshot.abs_path.clone();
         let path: Arc<Path> = Arc::from(path);
         let path_string = path.to_string_lossy().to_string();
@@ -1480,7 +1506,7 @@ impl RemoteWorktree {
             let response = rpc
                 .request(proto::OpenBuffer {
                     project_id,
-                    worktree_id: remote_worktree_id as u64,
+                    worktree_id: remote_worktree_id.to_proto(),
                     path: path_string,
                 })
                 .await?;
@@ -1575,14 +1601,14 @@ impl RemoteBuffer {
 }
 
 impl Snapshot {
-    pub fn id(&self) -> usize {
+    pub fn id(&self) -> WorktreeId {
         self.id
     }
 
     pub fn to_proto(&self) -> proto::Worktree {
         let root_name = self.root_name.clone();
         proto::Worktree {
-            id: self.id as u64,
+            id: self.id.0 as u64,
             root_name,
             entries: self
                 .entries_by_path
@@ -2003,7 +2029,7 @@ impl language::File for File {
         version: clock::Global,
         cx: &mut MutableAppContext,
     ) -> Task<Result<(clock::Global, SystemTime)>> {
-        let worktree_id = self.worktree.read(cx).id() as u64;
+        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();
@@ -2066,7 +2092,7 @@ 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.remote_id;
+                let worktree_id = worktree.id().to_proto();
                 let rpc = worktree.client.clone();
                 cx.background()
                     .spawn(async move {
@@ -2096,7 +2122,7 @@ impl File {
         file.and_then(|f| f.as_any().downcast_ref())
     }
 
-    pub fn worktree_id(&self, cx: &AppContext) -> usize {
+    pub fn worktree_id(&self, cx: &AppContext) -> WorktreeId {
         self.worktree.read(cx).id()
     }
 }
@@ -4055,7 +4081,7 @@ mod tests {
         let fs = Arc::new(RealFs);
         let next_entry_id = Arc::new(AtomicUsize::new(0));
         let mut initial_snapshot = Snapshot {
-            id: 0,
+            id: WorktreeId::from_usize(0),
             scan_id: 0,
             abs_path: root_dir.path().into(),
             entries_by_path: Default::default(),

crates/project_panel/src/project_panel.rs 🔗

@@ -14,7 +14,7 @@ use gpui::{
     ViewContext, ViewHandle, WeakViewHandle,
 };
 use postage::watch;
-use project::{Project, ProjectEntry, ProjectPath, Worktree};
+use project::{Project, ProjectEntry, ProjectPath, Worktree, WorktreeId};
 use std::{
     collections::{hash_map, HashMap},
     ffi::OsStr,
@@ -26,7 +26,7 @@ pub struct ProjectPanel {
     project: ModelHandle<Project>,
     list: UniformListState,
     visible_entries: Vec<Vec<usize>>,
-    expanded_dir_ids: HashMap<usize, Vec<usize>>,
+    expanded_dir_ids: HashMap<WorktreeId, Vec<usize>>,
     selection: Option<Selection>,
     settings: watch::Receiver<Settings>,
     handle: WeakViewHandle<Self>,
@@ -34,7 +34,7 @@ pub struct ProjectPanel {
 
 #[derive(Copy, Clone)]
 struct Selection {
-    worktree_id: usize,
+    worktree_id: WorktreeId,
     entry_id: usize,
     index: usize,
 }
@@ -67,7 +67,10 @@ pub fn init(cx: &mut MutableAppContext) {
 }
 
 pub enum Event {
-    OpenedEntry { worktree_id: usize, entry_id: usize },
+    OpenedEntry {
+        worktree_id: WorktreeId,
+        entry_id: usize,
+    },
 }
 
 impl ProjectPanel {
@@ -114,16 +117,16 @@ impl ProjectPanel {
             this
         });
         cx.subscribe(&project_panel, move |workspace, _, event, cx| match event {
-            Event::OpenedEntry {
+            &Event::OpenedEntry {
                 worktree_id,
                 entry_id,
             } => {
-                if let Some(worktree) = project.read(cx).worktree_for_id(*worktree_id, cx) {
-                    if let Some(entry) = worktree.read(cx).entry_for_id(*entry_id) {
+                if let Some(worktree) = project.read(cx).worktree_for_id(worktree_id, cx) {
+                    if let Some(entry) = worktree.read(cx).entry_for_id(entry_id) {
                         workspace
                             .open_entry(
                                 ProjectPath {
-                                    worktree_id: worktree.id(),
+                                    worktree_id,
                                     path: entry.path.clone(),
                                 },
                                 cx,
@@ -259,8 +262,8 @@ impl ProjectPanel {
 
     fn select_first(&mut self, cx: &mut ViewContext<Self>) {
         if let Some(worktree) = self.project.read(cx).worktrees().first() {
-            let worktree_id = worktree.id();
             let worktree = worktree.read(cx);
+            let worktree_id = worktree.id();
             if let Some(root_entry) = worktree.root_entry() {
                 self.selection = Some(Selection {
                     worktree_id,
@@ -313,7 +316,7 @@ impl ProjectPanel {
 
     fn update_visible_entries(
         &mut self,
-        new_selected_entry: Option<(usize, usize)>,
+        new_selected_entry: Option<(WorktreeId, usize)>,
         cx: &mut ViewContext<Self>,
     ) {
         let worktrees = self.project.read(cx).worktrees();
@@ -322,7 +325,7 @@ impl ProjectPanel {
         let mut entry_ix = 0;
         for worktree in worktrees {
             let snapshot = worktree.read(cx).snapshot();
-            let worktree_id = worktree.id();
+            let worktree_id = snapshot.id();
 
             let expanded_dir_ids = match self.expanded_dir_ids.entry(worktree_id) {
                 hash_map::Entry::Occupied(e) => e.into_mut(),
@@ -342,7 +345,7 @@ impl ProjectPanel {
             while let Some(item) = entry_iter.entry() {
                 visible_worktree_entries.push(entry_iter.offset());
                 if let Some(new_selected_entry) = new_selected_entry {
-                    if new_selected_entry == (worktree.id(), item.id) {
+                    if new_selected_entry == (worktree_id, item.id) {
                         self.selection = Some(Selection {
                             worktree_id,
                             entry_id: item.id,
@@ -371,7 +374,12 @@ impl ProjectPanel {
         }
     }
 
-    fn expand_entry(&mut self, worktree_id: usize, entry_id: usize, cx: &mut ViewContext<Self>) {
+    fn expand_entry(
+        &mut self,
+        worktree_id: WorktreeId,
+        entry_id: usize,
+        cx: &mut ViewContext<Self>,
+    ) {
         let project = self.project.read(cx);
         if let Some((worktree, expanded_dir_ids)) = project
             .worktree_for_id(worktree_id, cx)
@@ -417,12 +425,12 @@ impl ProjectPanel {
 
             let end_ix = range.end.min(ix + visible_worktree_entries.len());
             let worktree = &worktrees[worktree_ix];
+            let snapshot = worktree.read(cx).snapshot();
             let expanded_entry_ids = self
                 .expanded_dir_ids
-                .get(&worktree.id())
+                .get(&snapshot.id())
                 .map(Vec::as_slice)
                 .unwrap_or(&[]);
-            let snapshot = worktree.read(cx).snapshot();
             let root_name = OsStr::new(snapshot.root_name());
             let mut cursor = snapshot.entries(false);
 
@@ -439,11 +447,11 @@ impl ProjectPanel {
                         is_dir: entry.is_dir(),
                         is_expanded: expanded_entry_ids.binary_search(&entry.id).is_ok(),
                         is_selected: self.selection.map_or(false, |e| {
-                            e.worktree_id == worktree.id() && e.entry_id == entry.id
+                            e.worktree_id == snapshot.id() && e.entry_id == entry.id
                         }),
                     };
                     let entry = ProjectEntry {
-                        worktree_id: worktree.id(),
+                        worktree_id: snapshot.id(),
                         entry_id: entry.id,
                     };
                     callback(entry, details, cx);
@@ -461,7 +469,7 @@ impl ProjectPanel {
     ) -> ElementBox {
         let is_dir = details.is_dir;
         MouseEventHandler::new::<Self, _, _, _>(
-            (entry.worktree_id, entry.entry_id),
+            (entry.worktree_id.to_usize(), entry.entry_id),
             cx,
             |state, _| {
                 let style = match (details.is_selected, state.hovered) {
@@ -516,7 +524,7 @@ impl ProjectPanel {
             if is_dir {
                 cx.dispatch_action(ToggleExpanded(entry))
             } else {
-                cx.dispatch_action(Open(entry))
+                cx.dispatch_action(Open(dbg!(entry)))
             }
         })
         .with_cursor_style(CursorStyle::PointingHand)

crates/workspace/src/workspace.rs 🔗

@@ -619,10 +619,10 @@ impl Workspace {
         cx: &mut ViewContext<Self>,
     ) -> Task<Result<ProjectPath>> {
         let entry = self.worktree_for_abs_path(abs_path, cx);
-        cx.spawn(|_, _| async move {
+        cx.spawn(|_, cx| async move {
             let (worktree, path) = entry.await?;
             Ok(ProjectPath {
-                worktree_id: worktree.id(),
+                worktree_id: worktree.read_with(&cx, |t, _| t.id()),
                 path: path.into(),
             })
         })
@@ -1264,7 +1264,7 @@ impl WorkspaceHandle for ViewHandle<Workspace> {
             .worktrees(cx)
             .iter()
             .flat_map(|worktree| {
-                let worktree_id = worktree.id();
+                let worktree_id = worktree.read(cx).id();
                 worktree.read(cx).files(true, 0).map(move |f| ProjectPath {
                     worktree_id,
                     path: f.path.clone(),

crates/zed/src/zed.rs 🔗

@@ -556,7 +556,7 @@ mod tests {
                 workspace
                     .open_entry(
                         ProjectPath {
-                            worktree_id: worktree.id(),
+                            worktree_id: worktree.read(cx).id(),
                             path: Path::new("the-new-name.rs").into(),
                         },
                         cx,