Always read project entry id from workspace::Item

Max Brunsfeld and Antonio Scandurra created

We cannot store a workspace item's project entry id separately,
since buffers' entry ids can change (for example when doing
a *save as*).

Co-Authored-By: Antonio Scandurra <me@as-cii.com>

Change summary

crates/diagnostics/src/diagnostics.rs |  4 +
crates/editor/src/items.rs            |  6 ++
crates/search/src/project_search.rs   |  4 +
crates/workspace/src/pane.rs          | 70 ++++++++++++++--------------
crates/workspace/src/workspace.rs     | 17 ++++--
5 files changed, 59 insertions(+), 42 deletions(-)

Detailed changes

crates/diagnostics/src/diagnostics.rs 🔗

@@ -450,6 +450,10 @@ impl workspace::Item for ProjectDiagnosticsEditor {
         None
     }
 
+    fn project_entry_id(&self, _: &AppContext) -> Option<project::ProjectEntryId> {
+        None
+    }
+
     fn navigate(&mut self, data: Box<dyn Any>, cx: &mut ViewContext<Self>) {
         self.editor
             .update(cx, |editor, cx| editor.navigate(data, cx));

crates/editor/src/items.rs 🔗

@@ -5,7 +5,7 @@ use gpui::{
     ViewContext, ViewHandle,
 };
 use language::{Bias, Buffer, Diagnostic, File as _};
-use project::{File, Project, ProjectPath};
+use project::{File, Project, ProjectEntryId, ProjectPath};
 use std::fmt::Write;
 use std::path::PathBuf;
 use text::{Point, Selection};
@@ -41,6 +41,10 @@ impl Item for Editor {
         })
     }
 
+    fn project_entry_id(&self, cx: &AppContext) -> Option<ProjectEntryId> {
+        File::from_dyn(self.buffer().read(cx).file(cx)).and_then(|file| file.project_entry_id(cx))
+    }
+
     fn clone_on_split(&self, cx: &mut ViewContext<Self>) -> Option<Self>
     where
         Self: Sized,

crates/search/src/project_search.rs 🔗

@@ -250,6 +250,10 @@ impl Item for ProjectSearchView {
         None
     }
 
+    fn project_entry_id(&self, _: &AppContext) -> Option<project::ProjectEntryId> {
+        None
+    }
+
     fn can_save(&self, _: &gpui::AppContext) -> bool {
         true
     }

crates/workspace/src/pane.rs 🔗

@@ -9,8 +9,8 @@ use gpui::{
     geometry::{rect::RectF, vector::vec2f},
     keymap::Binding,
     platform::{CursorStyle, NavigationDirection},
-    AnyViewHandle, Entity, MutableAppContext, Quad, RenderContext, Task, View, ViewContext,
-    ViewHandle, WeakViewHandle,
+    AnyViewHandle, AppContext, Entity, MutableAppContext, Quad, RenderContext, Task, View,
+    ViewContext, ViewHandle, WeakViewHandle,
 };
 use project::{ProjectEntryId, ProjectPath};
 use std::{
@@ -99,7 +99,7 @@ pub enum Event {
 }
 
 pub struct Pane {
-    items: Vec<(Option<ProjectEntryId>, Box<dyn ItemHandle>)>,
+    items: Vec<Box<dyn ItemHandle>>,
     active_item_index: usize,
     nav_history: Rc<RefCell<NavHistory>>,
     toolbars: HashMap<TypeId, Box<dyn ToolbarHandle>>,
@@ -110,8 +110,7 @@ pub struct Pane {
 pub(crate) struct FollowerState {
     pub(crate) leader_id: PeerId,
     pub(crate) current_view_id: usize,
-    pub(crate) items_by_leader_view_id:
-        HashMap<usize, (Option<ProjectEntryId>, Box<dyn ItemHandle>)>,
+    pub(crate) items_by_leader_view_id: HashMap<usize, Box<dyn ItemHandle>>,
 }
 
 pub trait Toolbar: View {
@@ -295,8 +294,8 @@ impl Pane {
         cx: &mut ViewContext<Self>,
         build_item: impl FnOnce(&mut MutableAppContext) -> Box<dyn ItemHandle>,
     ) -> Box<dyn ItemHandle> {
-        for (ix, (existing_entry_id, item)) in self.items.iter().enumerate() {
-            if *existing_entry_id == Some(project_entry_id) {
+        for (ix, item) in self.items.iter().enumerate() {
+            if item.project_entry_id(cx) == Some(project_entry_id) {
                 let item = item.boxed_clone();
                 self.activate_item(ix, cx);
                 return item;
@@ -304,20 +303,15 @@ impl Pane {
         }
 
         let item = build_item(cx);
-        self.add_item(Some(project_entry_id), item.boxed_clone(), cx);
+        self.add_item(item.boxed_clone(), cx);
         item
     }
 
-    pub(crate) fn add_item(
-        &mut self,
-        project_entry_id: Option<ProjectEntryId>,
-        mut item: Box<dyn ItemHandle>,
-        cx: &mut ViewContext<Self>,
-    ) {
+    pub(crate) fn add_item(&mut self, mut item: Box<dyn ItemHandle>, cx: &mut ViewContext<Self>) {
         item.set_nav_history(self.nav_history.clone(), cx);
         item.added_to_pane(cx);
         let item_idx = cmp::min(self.active_item_index + 1, self.items.len());
-        self.items.insert(item_idx, (project_entry_id, item));
+        self.items.insert(item_idx, item);
         self.activate_item(item_idx, cx);
         cx.notify();
     }
@@ -328,39 +322,45 @@ impl Pane {
         cx: &mut ViewContext<Self>,
     ) -> Result<()> {
         let current_view_id = follower_state.current_view_id as usize;
-        let (project_entry_id, item) = follower_state
+        let item = follower_state
             .items_by_leader_view_id
             .get(&current_view_id)
             .ok_or_else(|| anyhow!("invalid current view id"))?
             .clone();
-        self.add_item(project_entry_id, item, cx);
+        self.add_item(item, cx);
         Ok(())
     }
 
     pub fn items(&self) -> impl Iterator<Item = &Box<dyn ItemHandle>> {
-        self.items.iter().map(|(_, view)| view)
+        self.items.iter()
     }
 
     pub fn active_item(&self) -> Option<Box<dyn ItemHandle>> {
-        self.items
-            .get(self.active_item_index)
-            .map(|(_, view)| view.clone())
+        self.items.get(self.active_item_index).cloned()
     }
 
-    pub fn project_entry_id_for_item(&self, item: &dyn ItemHandle) -> Option<ProjectEntryId> {
-        self.items.iter().find_map(|(entry_id, existing)| {
+    pub fn project_entry_id_for_item(
+        &self,
+        item: &dyn ItemHandle,
+        cx: &AppContext,
+    ) -> Option<ProjectEntryId> {
+        self.items.iter().find_map(|existing| {
             if existing.id() == item.id() {
-                *entry_id
+                existing.project_entry_id(cx)
             } else {
                 None
             }
         })
     }
 
-    pub fn item_for_entry(&self, entry_id: ProjectEntryId) -> Option<Box<dyn ItemHandle>> {
-        self.items.iter().find_map(|(id, view)| {
-            if *id == Some(entry_id) {
-                Some(view.boxed_clone())
+    pub fn item_for_entry(
+        &self,
+        entry_id: ProjectEntryId,
+        cx: &AppContext,
+    ) -> Option<Box<dyn ItemHandle>> {
+        self.items.iter().find_map(|item| {
+            if item.project_entry_id(cx) == Some(entry_id) {
+                Some(item.boxed_clone())
             } else {
                 None
             }
@@ -368,7 +368,7 @@ impl Pane {
     }
 
     pub fn index_for_item(&self, item: &dyn ItemHandle) -> Option<usize> {
-        self.items.iter().position(|(_, i)| i.id() == item.id())
+        self.items.iter().position(|i| i.id() == item.id())
     }
 
     pub fn activate_item(&mut self, index: usize, cx: &mut ViewContext<Self>) {
@@ -377,7 +377,7 @@ impl Pane {
             if prev_active_item_ix != self.active_item_index
                 && prev_active_item_ix < self.items.len()
             {
-                self.items[prev_active_item_ix].1.deactivated(cx);
+                self.items[prev_active_item_ix].deactivated(cx);
             }
             self.update_active_toolbar(cx);
             self.focus_active_item(cx);
@@ -408,13 +408,13 @@ impl Pane {
 
     pub fn close_active_item(&mut self, cx: &mut ViewContext<Self>) {
         if !self.items.is_empty() {
-            self.close_item(self.items[self.active_item_index].1.id(), cx)
+            self.close_item(self.items[self.active_item_index].id(), cx)
         }
     }
 
     pub fn close_inactive_items(&mut self, cx: &mut ViewContext<Self>) {
         if !self.items.is_empty() {
-            let active_item_id = self.items[self.active_item_index].1.id();
+            let active_item_id = self.items[self.active_item_index].id();
             self.close_items(cx, |id| id != active_item_id);
         }
     }
@@ -430,7 +430,7 @@ impl Pane {
     ) {
         let mut item_ix = 0;
         let mut new_active_item_index = self.active_item_index;
-        self.items.retain(|(_, item)| {
+        self.items.retain(|item| {
             if should_close(item.id()) {
                 if item_ix == self.active_item_index {
                     item.deactivated(cx);
@@ -529,7 +529,7 @@ impl Pane {
     fn update_active_toolbar(&mut self, cx: &mut ViewContext<Self>) {
         let active_item = self.items.get(self.active_item_index);
         for (toolbar_type_id, toolbar) in &self.toolbars {
-            let visible = toolbar.active_item_changed(active_item.map(|i| i.1.clone()), cx);
+            let visible = toolbar.active_item_changed(active_item.cloned(), cx);
             if Some(*toolbar_type_id) == self.active_toolbar_type {
                 self.active_toolbar_visible = visible;
             }
@@ -542,7 +542,7 @@ impl Pane {
         enum Tabs {}
         let tabs = MouseEventHandler::new::<Tabs, _, _>(0, cx, |mouse_state, cx| {
             let mut row = Flex::row();
-            for (ix, (_, item)) in self.items.iter().enumerate() {
+            for (ix, item) in self.items.iter().enumerate() {
                 let is_active = ix == self.active_item_index;
 
                 row.add_child({

crates/workspace/src/workspace.rs 🔗

@@ -53,7 +53,7 @@ type FollowedItemBuilders = Vec<
         ModelHandle<Project>,
         &mut Option<proto::view::Variant>,
         &mut MutableAppContext,
-    ) -> Option<Task<Result<(Option<ProjectEntryId>, Box<dyn ItemHandle>)>>>,
+    ) -> Option<Task<Result<Box<dyn ItemHandle>>>>,
 >;
 
 action!(Open, Arc<AppState>);
@@ -157,6 +157,7 @@ pub trait Item: View {
     fn navigate(&mut self, _: Box<dyn Any>, _: &mut ViewContext<Self>) {}
     fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
+    fn project_entry_id(&self, cx: &AppContext) -> Option<ProjectEntryId>;
     fn set_nav_history(&mut self, _: ItemNavHistory, _: &mut ViewContext<Self>);
     fn clone_on_split(&self, _: &mut ViewContext<Self>) -> Option<Self>
     where
@@ -224,12 +225,13 @@ pub trait FollowedItem: Item {
         project: ModelHandle<Project>,
         state: &mut Option<proto::view::Variant>,
         cx: &mut MutableAppContext,
-    ) -> Option<Task<Result<(Option<ProjectEntryId>, Box<dyn ItemHandle>)>>>;
+    ) -> Option<Task<Result<Box<dyn ItemHandle>>>>;
 }
 
 pub trait ItemHandle: 'static {
     fn tab_content(&self, style: &theme::Tab, cx: &AppContext) -> ElementBox;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
+    fn project_entry_id(&self, cx: &AppContext) -> Option<ProjectEntryId>;
     fn boxed_clone(&self) -> Box<dyn ItemHandle>;
     fn set_nav_history(&self, nav_history: Rc<RefCell<NavHistory>>, cx: &mut MutableAppContext);
     fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option<Box<dyn ItemHandle>>;
@@ -277,6 +279,10 @@ impl<T: Item> ItemHandle for ViewHandle<T> {
         self.read(cx).project_path(cx)
     }
 
+    fn project_entry_id(&self, cx: &AppContext) -> Option<ProjectEntryId> {
+        self.read(cx).project_entry_id(cx)
+    }
+
     fn boxed_clone(&self) -> Box<dyn ItemHandle> {
         Box::new(self.clone())
     }
@@ -814,7 +820,7 @@ impl Workspace {
 
     pub fn add_item(&mut self, item: Box<dyn ItemHandle>, cx: &mut ViewContext<Self>) {
         self.active_pane()
-            .update(cx, |pane, cx| pane.add_item(None, item, cx))
+            .update(cx, |pane, cx| pane.add_item(item, cx))
     }
 
     pub fn open_path(
@@ -877,7 +883,7 @@ impl Workspace {
         if let Some(item) = project_item
             .read(cx)
             .entry_id(cx)
-            .and_then(|entry_id| self.active_pane().read(cx).item_for_entry(entry_id))
+            .and_then(|entry_id| self.active_pane().read(cx).item_for_entry(entry_id, cx))
             .and_then(|item| item.downcast())
         {
             self.activate_item(&item, cx);
@@ -959,10 +965,9 @@ impl Workspace {
         let new_pane = self.add_pane(cx);
         self.activate_pane(new_pane.clone(), cx);
         if let Some(item) = pane.read(cx).active_item() {
-            let project_entry_id = pane.read(cx).project_entry_id_for_item(item.as_ref());
             if let Some(clone) = item.clone_on_split(cx.as_mut()) {
                 new_pane.update(cx, |new_pane, cx| {
-                    new_pane.add_item(project_entry_id, clone, cx);
+                    new_pane.add_item(clone, cx);
                 });
             }
         }