Refactor opening workspace items

Antonio Scandurra and Nathan Sobo created

Co-Authored-By: Nathan Sobo <nathan@zed.dev>

Change summary

Cargo.lock                                |   2 
crates/diagnostics/src/diagnostics.rs     |  55 +++--
crates/editor/src/editor.rs               |   6 
crates/editor/src/items.rs                |  58 +++--
crates/file_finder/src/file_finder.rs     |   8 
crates/gpui/src/app.rs                    |   4 
crates/gpui/src/executor.rs               |  15 +
crates/project_panel/Cargo.toml           |   1 
crates/project_panel/src/project_panel.rs |   2 
crates/workspace/Cargo.toml               |   1 
crates/workspace/src/pane.rs              | 109 +++++------
crates/workspace/src/workspace.rs         | 226 ++++++++++++------------
crates/zed/src/main.rs                    |   6 
crates/zed/src/test.rs                    |   6 
crates/zed/src/zed.rs                     |  37 +--
15 files changed, 280 insertions(+), 256 deletions(-)

Detailed changes

Cargo.lock 🔗

@@ -3502,6 +3502,7 @@ dependencies = [
  "project",
  "serde_json",
  "theme",
+ "util",
  "workspace",
 ]
 
@@ -5650,6 +5651,7 @@ dependencies = [
  "anyhow",
  "client",
  "clock",
+ "collections",
  "gpui",
  "language",
  "log",

crates/diagnostics/src/diagnostics.rs 🔗

@@ -43,7 +43,7 @@ struct ProjectDiagnostics {
 }
 
 struct ProjectDiagnosticsEditor {
-    project: ModelHandle<Project>,
+    model: ModelHandle<ProjectDiagnostics>,
     editor: ViewHandle<Editor>,
     excerpts: ModelHandle<MultiBuffer>,
     path_states: Vec<(Arc<Path>, Vec<DiagnosticGroupState>)>,
@@ -109,10 +109,11 @@ impl View for ProjectDiagnosticsEditor {
 
 impl ProjectDiagnosticsEditor {
     fn new(
-        project: ModelHandle<Project>,
+        model: ModelHandle<ProjectDiagnostics>,
         settings: watch::Receiver<workspace::Settings>,
         cx: &mut ViewContext<Self>,
     ) -> Self {
+        let project = model.read(cx).project.clone();
         cx.subscribe(&project, |this, _, event, cx| match event {
             project::Event::DiskBasedDiagnosticsUpdated { worktree_id } => {
                 if let Some(paths) = this.paths_to_update.remove(&worktree_id) {
@@ -142,7 +143,7 @@ impl ProjectDiagnosticsEditor {
             .map(|e| e.0)
             .collect();
         let this = Self {
-            project,
+            model,
             excerpts,
             editor,
             build_settings,
@@ -161,7 +162,7 @@ impl ProjectDiagnosticsEditor {
 
     fn deploy(workspace: &mut Workspace, _: &Deploy, cx: &mut ViewContext<Workspace>) {
         let diagnostics = cx.add_model(|_| ProjectDiagnostics::new(workspace.project().clone()));
-        workspace.add_item(diagnostics, cx);
+        workspace.open_item(diagnostics, cx);
     }
 
     fn open_excerpts(&mut self, _: &OpenExcerpts, cx: &mut ViewContext<Self>) {
@@ -183,7 +184,7 @@ impl ProjectDiagnosticsEditor {
     }
 
     fn update_excerpts(&self, paths: HashSet<ProjectPath>, cx: &mut ViewContext<Self>) {
-        let project = self.project.clone();
+        let project = self.model.read(cx).project.clone();
         cx.spawn(|this, mut cx| {
             async move {
                 for path in paths {
@@ -461,8 +462,7 @@ impl workspace::Item for ProjectDiagnostics {
         settings: watch::Receiver<workspace::Settings>,
         cx: &mut ViewContext<Self::View>,
     ) -> Self::View {
-        let project = handle.read(cx).project.clone();
-        ProjectDiagnosticsEditor::new(project, settings, cx)
+        ProjectDiagnosticsEditor::new(handle, settings, cx)
     }
 
     fn project_path(&self) -> Option<project::ProjectPath> {
@@ -471,6 +471,12 @@ impl workspace::Item for ProjectDiagnostics {
 }
 
 impl workspace::ItemView for ProjectDiagnosticsEditor {
+    type ItemHandle = ModelHandle<ProjectDiagnostics>;
+
+    fn item_handle(&self, _: &AppContext) -> Self::ItemHandle {
+        self.model.clone()
+    }
+
     fn title(&self, _: &AppContext) -> String {
         "Project Diagnostics".to_string()
     }
@@ -479,10 +485,26 @@ impl workspace::ItemView for ProjectDiagnosticsEditor {
         None
     }
 
+    fn is_dirty(&self, cx: &AppContext) -> bool {
+        self.excerpts.read(cx).read(cx).is_dirty()
+    }
+
+    fn has_conflict(&self, cx: &AppContext) -> bool {
+        self.excerpts.read(cx).read(cx).has_conflict()
+    }
+
+    fn can_save(&self, _: &AppContext) -> bool {
+        true
+    }
+
     fn save(&mut self, cx: &mut ViewContext<Self>) -> Result<Task<Result<()>>> {
         self.excerpts.update(cx, |excerpts, cx| excerpts.save(cx))
     }
 
+    fn can_save_as(&self, _: &AppContext) -> bool {
+        false
+    }
+
     fn save_as(
         &mut self,
         _: ModelHandle<project::Worktree>,
@@ -492,28 +514,12 @@ impl workspace::ItemView for ProjectDiagnosticsEditor {
         unreachable!()
     }
 
-    fn is_dirty(&self, cx: &AppContext) -> bool {
-        self.excerpts.read(cx).read(cx).is_dirty()
-    }
-
-    fn has_conflict(&self, cx: &AppContext) -> bool {
-        self.excerpts.read(cx).read(cx).has_conflict()
-    }
-
     fn should_update_tab_on_event(event: &Event) -> bool {
         matches!(
             event,
             Event::Saved | Event::Dirtied | Event::FileHandleChanged
         )
     }
-
-    fn can_save(&self, _: &AppContext) -> bool {
-        true
-    }
-
-    fn can_save_as(&self, _: &AppContext) -> bool {
-        false
-    }
 }
 
 fn compare_diagnostics<L: language::ToOffset, R: language::ToOffset>(
@@ -677,8 +683,9 @@ mod tests {
                 .unwrap();
         });
 
+        let model = cx.add_model(|_| ProjectDiagnostics::new(project.clone()));
         let view = cx.add_view(Default::default(), |cx| {
-            ProjectDiagnosticsEditor::new(project.clone(), settings, cx)
+            ProjectDiagnosticsEditor::new(model, settings, cx)
         });
 
         view.condition(&mut cx, |view, cx| view.text(cx).contains("fn main()"))

crates/editor/src/editor.rs 🔗

@@ -111,8 +111,8 @@ action!(FoldSelectedRanges);
 action!(Scroll, Vector2F);
 action!(Select, SelectPhase);
 
-pub fn init(cx: &mut MutableAppContext, entry_openers: &mut Vec<Box<dyn EntryOpener>>) {
-    entry_openers.push(Box::new(items::BufferOpener));
+pub fn init(cx: &mut MutableAppContext, path_openers: &mut Vec<Box<dyn EntryOpener>>) {
+    path_openers.push(Box::new(items::BufferOpener));
     cx.add_bindings(vec![
         Binding::new("escape", Cancel, Some("Editor")),
         Binding::new("backspace", Backspace, Some("Editor")),
@@ -525,7 +525,7 @@ impl Editor {
             Buffer::new(0, "", cx).with_language(Some(language::PLAIN_TEXT.clone()), None, cx)
         });
         let buffer = cx.add_model(|cx| MultiBuffer::singleton(buffer, cx));
-        workspace.add_item(BufferItemHandle(buffer), cx);
+        workspace.open_item(BufferItemHandle(buffer), cx);
     }
 
     pub fn replica_id(&self, cx: &AppContext) -> ReplicaId {

crates/editor/src/items.rs 🔗

@@ -71,6 +71,10 @@ impl ItemHandle for BufferItemHandle {
             path: f.path().clone(),
         })
     }
+
+    fn id(&self) -> usize {
+        self.0.id()
+    }
 }
 
 impl WeakItemHandle for WeakBufferItemHandle {
@@ -79,22 +83,17 @@ impl WeakItemHandle for WeakBufferItemHandle {
             .upgrade(cx)
             .map(|buffer| Box::new(BufferItemHandle(buffer)) as Box<dyn ItemHandle>)
     }
-}
 
-impl ItemView for Editor {
-    fn should_activate_item_on_event(event: &Event) -> bool {
-        matches!(event, Event::Activate)
+    fn id(&self) -> usize {
+        self.0.id()
     }
+}
 
-    fn should_close_item_on_event(event: &Event) -> bool {
-        matches!(event, Event::Closed)
-    }
+impl ItemView for Editor {
+    type ItemHandle = BufferItemHandle;
 
-    fn should_update_tab_on_event(event: &Event) -> bool {
-        matches!(
-            event,
-            Event::Saved | Event::Dirtied | Event::FileHandleChanged
-        )
+    fn item_handle(&self, cx: &AppContext) -> Self::ItemHandle {
+        BufferItemHandle(self.buffer.clone())
     }
 
     fn title(&self, cx: &AppContext) -> String {
@@ -124,6 +123,18 @@ impl ItemView for Editor {
         Some(self.clone(cx))
     }
 
+    fn is_dirty(&self, cx: &AppContext) -> bool {
+        self.buffer().read(cx).read(cx).is_dirty()
+    }
+
+    fn has_conflict(&self, cx: &AppContext) -> bool {
+        self.buffer().read(cx).read(cx).has_conflict()
+    }
+
+    fn can_save(&self, cx: &AppContext) -> bool {
+        self.project_path(cx).is_some()
+    }
+
     fn save(&mut self, cx: &mut ViewContext<Self>) -> Result<Task<Result<()>>> {
         let save = self.buffer().update(cx, |b, cx| b.save(cx))?;
         Ok(cx.spawn(|_, _| async move {
@@ -132,6 +143,10 @@ impl ItemView for Editor {
         }))
     }
 
+    fn can_save_as(&self, _: &AppContext) -> bool {
+        true
+    }
+
     fn save_as(
         &mut self,
         worktree: ModelHandle<Worktree>,
@@ -180,20 +195,19 @@ impl ItemView for Editor {
         })
     }
 
-    fn is_dirty(&self, cx: &AppContext) -> bool {
-        self.buffer().read(cx).read(cx).is_dirty()
-    }
-
-    fn has_conflict(&self, cx: &AppContext) -> bool {
-        self.buffer().read(cx).read(cx).has_conflict()
+    fn should_activate_item_on_event(event: &Event) -> bool {
+        matches!(event, Event::Activate)
     }
 
-    fn can_save(&self, cx: &AppContext) -> bool {
-        self.project_path(cx).is_some()
+    fn should_close_item_on_event(event: &Event) -> bool {
+        matches!(event, Event::Closed)
     }
 
-    fn can_save_as(&self, _: &AppContext) -> bool {
-        true
+    fn should_update_tab_on_event(event: &Event) -> bool {
+        matches!(
+            event,
+            Event::Saved | Event::Dirtied | Event::FileHandleChanged
+        )
     }
 }
 

crates/file_finder/src/file_finder.rs 🔗

@@ -251,7 +251,7 @@ impl FileFinder {
             Event::Selected(project_path) => {
                 workspace
                     .open_path(project_path.clone(), cx)
-                    .map(|d| d.detach());
+                    .detach_and_log_err(cx);
                 workspace.dismiss_modal(cx);
             }
             Event::Dismissed => {
@@ -431,14 +431,14 @@ mod tests {
 
     #[gpui::test]
     async fn test_matching_paths(mut cx: gpui::TestAppContext) {
-        let mut entry_openers = Vec::new();
+        let mut path_openers = Vec::new();
         cx.update(|cx| {
             super::init(cx);
-            editor::init(cx, &mut entry_openers);
+            editor::init(cx, &mut path_openers);
         });
 
         let mut params = cx.update(WorkspaceParams::test);
-        params.entry_openers = Arc::from(entry_openers);
+        params.path_openers = Arc::from(path_openers);
         params
             .fs
             .as_fake()

crates/gpui/src/app.rs 🔗

@@ -2773,6 +2773,10 @@ impl<T: Entity> WeakModelHandle<T> {
         }
     }
 
+    pub fn id(&self) -> usize {
+        self.model_id
+    }
+
     pub fn upgrade(self, cx: &impl UpgradeModelHandle) -> Option<ModelHandle<T>> {
         cx.upgrade_model_handle(self)
     }

crates/gpui/src/executor.rs 🔗

@@ -7,7 +7,7 @@ use rand::prelude::*;
 use smol::{channel, prelude::*, Executor, Timer};
 use std::{
     any::Any,
-    fmt::{self, Debug},
+    fmt::{self, Debug, Display},
     marker::PhantomData,
     mem,
     ops::RangeInclusive,
@@ -25,7 +25,7 @@ use waker_fn::waker_fn;
 
 use crate::{
     platform::{self, Dispatcher},
-    util,
+    util, MutableAppContext,
 };
 
 pub enum Foreground {
@@ -682,6 +682,17 @@ impl<T> Task<T> {
     }
 }
 
+impl<T: 'static, E: 'static + Display> Task<Result<T, E>> {
+    pub fn detach_and_log_err(self, cx: &mut MutableAppContext) {
+        cx.spawn(|_| async move {
+            if let Err(err) = self.await {
+                log::error!("{}", err);
+            }
+        })
+        .detach();
+    }
+}
+
 impl<T: Send> Task<T> {
     fn send(any_task: AnyTask) -> Self {
         Self::Send {

crates/project_panel/Cargo.toml 🔗

@@ -10,6 +10,7 @@ path = "src/project_panel.rs"
 gpui = { path = "../gpui" }
 project = { path = "../project" }
 theme = { path = "../theme" }
+util = { path = "../util" }
 workspace = { path = "../workspace" }
 postage = { version = "0.4.1", features = ["futures-traits"] }
 

crates/workspace/Cargo.toml 🔗

@@ -12,6 +12,7 @@ test-support = ["client/test-support", "project/test-support"]
 [dependencies]
 client = { path = "../client" }
 clock = { path = "../clock" }
+collections = { path = "../collections" }
 gpui = { path = "../gpui" }
 language = { path = "../language" }
 project = { path = "../project" }

crates/workspace/src/pane.rs 🔗

@@ -1,15 +1,14 @@
 use super::{ItemViewHandle, SplitDirection};
-use crate::Settings;
+use crate::{ItemHandle, Settings};
 use gpui::{
     action,
     elements::*,
     geometry::{rect::RectF, vector::vec2f},
     keymap::Binding,
     platform::CursorStyle,
-    Entity, MutableAppContext, Quad, RenderContext, View, ViewContext, ViewHandle,
+    Entity, MutableAppContext, Quad, RenderContext, View, ViewContext,
 };
 use postage::watch;
-use project::ProjectPath;
 use std::cmp;
 
 action!(Split, SplitDirection);
@@ -70,7 +69,7 @@ pub struct TabState {
 }
 
 pub struct Pane {
-    items: Vec<Box<dyn ItemViewHandle>>,
+    item_views: Vec<Box<dyn ItemViewHandle>>,
     active_item: usize,
     settings: watch::Receiver<Settings>,
 }
@@ -78,7 +77,7 @@ pub struct Pane {
 impl Pane {
     pub fn new(settings: watch::Receiver<Settings>) -> Self {
         Self {
-            items: Vec::new(),
+            item_views: Vec::new(),
             active_item: 0,
             settings,
         }
@@ -88,43 +87,53 @@ impl Pane {
         cx.emit(Event::Activate);
     }
 
-    pub fn add_item(&mut self, item: Box<dyn ItemViewHandle>, cx: &mut ViewContext<Self>) -> usize {
-        let item_idx = cmp::min(self.active_item + 1, self.items.len());
-        self.items.insert(item_idx, item);
-        cx.notify();
-        item_idx
+    pub fn open_item<T>(
+        &mut self,
+        item_handle: T,
+        cx: &mut ViewContext<Self>,
+    ) -> Box<dyn ItemViewHandle>
+    where
+        T: 'static + ItemHandle,
+    {
+        for (ix, item_view) in self.item_views.iter().enumerate() {
+            if item_view.item_handle(cx).id() == item_handle.id() {
+                let item_view = item_view.boxed_clone();
+                self.activate_item(ix, cx);
+                return item_view;
+            }
+        }
+
+        let item_view = item_handle.add_view(cx.window_id(), self.settings.clone(), cx);
+        self.add_item_view(item_view.boxed_clone(), cx);
+        item_view
     }
 
-    pub fn items(&self) -> &[Box<dyn ItemViewHandle>] {
-        &self.items
+    pub fn add_item_view(
+        &mut self,
+        item_view: Box<dyn ItemViewHandle>,
+        cx: &mut ViewContext<Self>,
+    ) {
+        item_view.added_to_pane(cx);
+        let item_idx = cmp::min(self.active_item + 1, self.item_views.len());
+        self.item_views.insert(item_idx, item_view);
+        self.activate_item(item_idx, cx);
+        cx.notify();
     }
 
-    pub fn active_item(&self) -> Option<Box<dyn ItemViewHandle>> {
-        self.items.get(self.active_item).cloned()
+    pub fn item_views(&self) -> &[Box<dyn ItemViewHandle>] {
+        &self.item_views
     }
 
-    pub fn activate_entry(
-        &mut self,
-        project_path: ProjectPath,
-        cx: &mut ViewContext<Self>,
-    ) -> Option<Box<dyn ItemViewHandle>> {
-        if let Some(index) = self.items.iter().position(|item| {
-            item.project_path(cx.as_ref())
-                .map_or(false, |item_path| item_path == project_path)
-        }) {
-            self.activate_item(index, cx);
-            self.items.get(index).map(|handle| handle.boxed_clone())
-        } else {
-            None
-        }
+    pub fn active_item(&self) -> Option<Box<dyn ItemViewHandle>> {
+        self.item_views.get(self.active_item).cloned()
     }
 
     pub fn item_index(&self, item: &dyn ItemViewHandle) -> Option<usize> {
-        self.items.iter().position(|i| i.id() == item.id())
+        self.item_views.iter().position(|i| i.id() == item.id())
     }
 
     pub fn activate_item(&mut self, index: usize, cx: &mut ViewContext<Self>) {
-        if index < self.items.len() {
+        if index < self.item_views.len() {
             self.active_item = index;
             self.focus_active_item(cx);
             cx.notify();
@@ -134,15 +143,15 @@ impl Pane {
     pub fn activate_prev_item(&mut self, cx: &mut ViewContext<Self>) {
         if self.active_item > 0 {
             self.active_item -= 1;
-        } else if self.items.len() > 0 {
-            self.active_item = self.items.len() - 1;
+        } else if self.item_views.len() > 0 {
+            self.active_item = self.item_views.len() - 1;
         }
         self.focus_active_item(cx);
         cx.notify();
     }
 
     pub fn activate_next_item(&mut self, cx: &mut ViewContext<Self>) {
-        if self.active_item + 1 < self.items.len() {
+        if self.active_item + 1 < self.item_views.len() {
             self.active_item += 1;
         } else {
             self.active_item = 0;
@@ -152,15 +161,15 @@ 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].id(), cx)
+        if !self.item_views.is_empty() {
+            self.close_item(self.item_views[self.active_item].id(), cx)
         }
     }
 
     pub fn close_item(&mut self, item_id: usize, cx: &mut ViewContext<Self>) {
-        self.items.retain(|item| item.id() != item_id);
-        self.active_item = cmp::min(self.active_item, self.items.len().saturating_sub(1));
-        if self.items.is_empty() {
+        self.item_views.retain(|item| item.id() != item_id);
+        self.active_item = cmp::min(self.active_item, self.item_views.len().saturating_sub(1));
+        if self.item_views.is_empty() {
             cx.emit(Event::Remove);
         }
         cx.notify();
@@ -183,11 +192,11 @@ impl Pane {
         enum Tabs {}
         let tabs = MouseEventHandler::new::<Tabs, _, _, _>(cx.view_id(), cx, |mouse_state, cx| {
             let mut row = Flex::row();
-            for (ix, item) in self.items.iter().enumerate() {
+            for (ix, item_view) in self.item_views.iter().enumerate() {
                 let is_active = ix == self.active_item;
 
                 row.add_child({
-                    let mut title = item.title(cx);
+                    let mut title = item_view.title(cx);
                     if title.len() > MAX_TAB_TITLE_LEN {
                         let mut truncated_len = MAX_TAB_TITLE_LEN;
                         while !title.is_char_boundary(truncated_len) {
@@ -212,9 +221,9 @@ impl Pane {
                                 .with_child(
                                     Align::new({
                                         let diameter = 7.0;
-                                        let icon_color = if item.has_conflict(cx) {
+                                        let icon_color = if item_view.has_conflict(cx) {
                                             Some(style.icon_conflict)
-                                        } else if item.is_dirty(cx) {
+                                        } else if item_view.is_dirty(cx) {
                                             Some(style.icon_dirty)
                                         } else {
                                             None
@@ -271,7 +280,7 @@ impl Pane {
                                 .with_child(
                                     Align::new(
                                         ConstrainedBox::new(if mouse_state.hovered {
-                                            let item_id = item.id();
+                                            let item_id = item_view.id();
                                             enum TabCloseButton {}
                                             let icon = Svg::new("icons/x.svg");
                                             MouseEventHandler::new::<TabCloseButton, _, _, _>(
@@ -354,17 +363,3 @@ impl View for Pane {
         self.focus_active_item(cx);
     }
 }
-
-pub trait PaneHandle {
-    fn add_item_view(&self, item: Box<dyn ItemViewHandle>, cx: &mut MutableAppContext);
-}
-
-impl PaneHandle for ViewHandle<Pane> {
-    fn add_item_view(&self, item: Box<dyn ItemViewHandle>, cx: &mut MutableAppContext) {
-        item.set_parent_pane(self, cx);
-        self.update(cx, |pane, cx| {
-            let item_idx = pane.add_item(item, cx);
-            pane.activate_item(item_idx, cx);
-        });
-    }
-}

crates/workspace/src/workspace.rs 🔗

@@ -7,6 +7,7 @@ mod status_bar;
 use anyhow::{anyhow, Result};
 use client::{Authenticate, ChannelList, Client, User, UserStore};
 use clock::ReplicaId;
+use collections::HashSet;
 use gpui::{
     action,
     color::Color,
@@ -32,6 +33,7 @@ use status_bar::StatusBar;
 pub use status_bar::StatusItemView;
 use std::{
     future::Future,
+    hash::{Hash, Hasher},
     path::{Path, PathBuf},
     sync::Arc,
 };
@@ -94,7 +96,7 @@ pub struct AppState {
     pub user_store: ModelHandle<client::UserStore>,
     pub fs: Arc<dyn fs::Fs>,
     pub channel_list: ModelHandle<client::ChannelList>,
-    pub entry_openers: Arc<[Box<dyn EntryOpener>]>,
+    pub path_openers: Arc<[Box<dyn EntryOpener>]>,
     pub build_window_options: &'static dyn Fn() -> WindowOptions<'static>,
     pub build_workspace: &'static dyn Fn(
         ModelHandle<Project>,
@@ -137,6 +139,9 @@ pub trait Item: Entity + Sized {
 }
 
 pub trait ItemView: View {
+    type ItemHandle: ItemHandle;
+
+    fn item_handle(&self, cx: &AppContext) -> Self::ItemHandle;
     fn title(&self, cx: &AppContext) -> String;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
     fn clone_on_split(&self, _: &mut ViewContext<Self>) -> Option<Self>
@@ -172,6 +177,7 @@ pub trait ItemView: View {
 }
 
 pub trait ItemHandle: Send + Sync {
+    fn id(&self) -> usize;
     fn add_view(
         &self,
         window_id: usize,
@@ -184,15 +190,17 @@ pub trait ItemHandle: Send + Sync {
 }
 
 pub trait WeakItemHandle {
+    fn id(&self) -> usize;
     fn upgrade(&self, cx: &AppContext) -> Option<Box<dyn ItemHandle>>;
 }
 
 pub trait ItemViewHandle {
+    fn item_handle(&self, cx: &AppContext) -> Box<dyn ItemHandle>;
     fn title(&self, cx: &AppContext) -> String;
     fn project_path(&self, cx: &AppContext) -> Option<ProjectPath>;
     fn boxed_clone(&self) -> Box<dyn ItemViewHandle>;
     fn clone_on_split(&self, cx: &mut MutableAppContext) -> Option<Box<dyn ItemViewHandle>>;
-    fn set_parent_pane(&self, pane: &ViewHandle<Pane>, cx: &mut MutableAppContext);
+    fn added_to_pane(&self, cx: &mut ViewContext<Pane>);
     fn id(&self) -> usize;
     fn to_any(&self) -> AnyViewHandle;
     fn is_dirty(&self, cx: &AppContext) -> bool;
@@ -209,6 +217,10 @@ pub trait ItemViewHandle {
 }
 
 impl<T: Item> ItemHandle for ModelHandle<T> {
+    fn id(&self) -> usize {
+        self.id()
+    }
+
     fn add_view(
         &self,
         window_id: usize,
@@ -232,6 +244,10 @@ impl<T: Item> ItemHandle for ModelHandle<T> {
 }
 
 impl ItemHandle for Box<dyn ItemHandle> {
+    fn id(&self) -> usize {
+        ItemHandle::id(self.as_ref())
+    }
+
     fn add_view(
         &self,
         window_id: usize,
@@ -255,12 +271,34 @@ impl ItemHandle for Box<dyn ItemHandle> {
 }
 
 impl<T: Item> WeakItemHandle for WeakModelHandle<T> {
+    fn id(&self) -> usize {
+        WeakModelHandle::id(self)
+    }
+
     fn upgrade(&self, cx: &AppContext) -> Option<Box<dyn ItemHandle>> {
         WeakModelHandle::<T>::upgrade(*self, cx).map(|i| Box::new(i) as Box<dyn ItemHandle>)
     }
 }
 
+impl Hash for Box<dyn WeakItemHandle> {
+    fn hash<H: Hasher>(&self, state: &mut H) {
+        self.id().hash(state);
+    }
+}
+
+impl PartialEq for Box<dyn WeakItemHandle> {
+    fn eq(&self, other: &Self) -> bool {
+        self.id() == other.id()
+    }
+}
+
+impl Eq for Box<dyn WeakItemHandle> {}
+
 impl<T: ItemView> ItemViewHandle for ViewHandle<T> {
+    fn item_handle(&self, cx: &AppContext) -> Box<dyn ItemHandle> {
+        Box::new(self.read(cx).item_handle(cx))
+    }
+
     fn title(&self, cx: &AppContext) -> String {
         self.read(cx).title(cx)
     }
@@ -280,25 +318,23 @@ impl<T: ItemView> ItemViewHandle for ViewHandle<T> {
         .map(|handle| Box::new(handle) as Box<dyn ItemViewHandle>)
     }
 
-    fn set_parent_pane(&self, pane: &ViewHandle<Pane>, cx: &mut MutableAppContext) {
-        pane.update(cx, |_, cx| {
-            cx.subscribe(self, |pane, item, event, cx| {
-                if T::should_close_item_on_event(event) {
-                    pane.close_item(item.id(), cx);
-                    return;
-                }
-                if T::should_activate_item_on_event(event) {
-                    if let Some(ix) = pane.item_index(&item) {
-                        pane.activate_item(ix, cx);
-                        pane.activate(cx);
-                    }
-                }
-                if T::should_update_tab_on_event(event) {
-                    cx.notify()
+    fn added_to_pane(&self, cx: &mut ViewContext<Pane>) {
+        cx.subscribe(self, |pane, item, event, cx| {
+            if T::should_close_item_on_event(event) {
+                pane.close_item(item.id(), cx);
+                return;
+            }
+            if T::should_activate_item_on_event(event) {
+                if let Some(ix) = pane.item_index(&item) {
+                    pane.activate_item(ix, cx);
+                    pane.activate(cx);
                 }
-            })
-            .detach();
-        });
+            }
+            if T::should_update_tab_on_event(event) {
+                cx.notify()
+            }
+        })
+        .detach();
     }
 
     fn save(&self, cx: &mut MutableAppContext) -> Result<Task<Result<()>>> {
@@ -360,7 +396,7 @@ pub struct WorkspaceParams {
     pub settings: watch::Receiver<Settings>,
     pub user_store: ModelHandle<UserStore>,
     pub channel_list: ModelHandle<ChannelList>,
-    pub entry_openers: Arc<[Box<dyn EntryOpener>]>,
+    pub path_openers: Arc<[Box<dyn EntryOpener>]>,
 }
 
 impl WorkspaceParams {
@@ -392,7 +428,7 @@ impl WorkspaceParams {
             languages,
             settings: watch::channel_with(settings).1,
             user_store,
-            entry_openers: Arc::from([]),
+            path_openers: Arc::from([]),
         }
     }
 
@@ -412,7 +448,7 @@ impl WorkspaceParams {
             settings: app_state.settings.clone(),
             user_store: app_state.user_store.clone(),
             channel_list: app_state.channel_list.clone(),
-            entry_openers: app_state.entry_openers.clone(),
+            path_openers: app_state.path_openers.clone(),
         }
     }
 }
@@ -430,8 +466,8 @@ pub struct Workspace {
     active_pane: ViewHandle<Pane>,
     status_bar: ViewHandle<StatusBar>,
     project: ModelHandle<Project>,
-    entry_openers: Arc<[Box<dyn EntryOpener>]>,
-    items: Vec<Box<dyn WeakItemHandle>>,
+    path_openers: Arc<[Box<dyn EntryOpener>]>,
+    items: HashSet<Box<dyn WeakItemHandle>>,
     _observe_current_user: Task<()>,
 }
 
@@ -484,7 +520,7 @@ impl Workspace {
             left_sidebar: Sidebar::new(Side::Left),
             right_sidebar: Sidebar::new(Side::Right),
             project: params.project.clone(),
-            entry_openers: params.entry_openers.clone(),
+            path_openers: params.path_openers.clone(),
             items: Default::default(),
             _observe_current_user,
         }
@@ -560,13 +596,13 @@ impl Workspace {
                     async move {
                         let project_path = project_path.await.ok()?;
                         if fs.is_file(&abs_path).await {
-                            if let Some(entry) =
+                            Some(
                                 this.update(&mut cx, |this, cx| this.open_path(project_path, cx))
-                            {
-                                return Some(entry.await);
-                            }
+                                    .await,
+                            )
+                        } else {
+                            None
                         }
-                        None
                     }
                 })
             })
@@ -667,102 +703,51 @@ impl Workspace {
     #[must_use]
     pub fn open_path(
         &mut self,
-        project_path: ProjectPath,
+        path: ProjectPath,
         cx: &mut ViewContext<Self>,
-    ) -> Option<Task<Result<Box<dyn ItemViewHandle>, Arc<anyhow::Error>>>> {
-        let pane = self.active_pane().clone();
-        if let Some(existing_item) =
-            self.activate_or_open_existing_entry(project_path.clone(), &pane, cx)
-        {
-            return Some(cx.foreground().spawn(async move { Ok(existing_item) }));
+    ) -> Task<Result<Box<dyn ItemViewHandle>, Arc<anyhow::Error>>> {
+        if let Some(existing_item) = self.item_for_path(&path, cx) {
+            return Task::ready(Ok(self.open_item(existing_item, cx)));
         }
 
-        let worktree = match self
-            .project
-            .read(cx)
-            .worktree_for_id(project_path.worktree_id, cx)
-        {
+        let worktree = match self.project.read(cx).worktree_for_id(path.worktree_id, cx) {
             Some(worktree) => worktree,
             None => {
-                log::error!("worktree {} does not exist", project_path.worktree_id);
-                return None;
+                return Task::ready(Err(Arc::new(anyhow!(
+                    "worktree {} does not exist",
+                    path.worktree_id
+                ))));
             }
         };
 
-        let project_path = project_path.clone();
-        let entry_openers = self.entry_openers.clone();
-        let task = worktree.update(cx, |worktree, cx| {
-            for opener in entry_openers.iter() {
+        let project_path = path.clone();
+        let path_openers = self.path_openers.clone();
+        let open_task = worktree.update(cx, |worktree, cx| {
+            for opener in path_openers.iter() {
                 if let Some(task) = opener.open(worktree, project_path.clone(), cx) {
-                    return Some(task);
+                    return task;
                 }
             }
-            log::error!("no opener for path {:?} found", project_path);
-            None
-        })?;
+            Task::ready(Err(anyhow!("no opener found for path {:?}", project_path)))
+        });
 
-        let pane = pane.downgrade();
-        Some(cx.spawn(|this, mut cx| async move {
-            let load_result = task.await;
+        let pane = self.active_pane().clone().downgrade();
+        cx.spawn(|this, mut cx| async move {
+            let item = open_task.await?;
             this.update(&mut cx, |this, cx| {
                 let pane = pane
                     .upgrade(&cx)
                     .ok_or_else(|| anyhow!("could not upgrade pane reference"))?;
-                let item = load_result?;
-
-                // By the time loading finishes, the entry could have been already added
-                // to the pane. If it was, we activate it, otherwise we'll store the
-                // item and add a new view for it.
-                if let Some(existing) =
-                    this.activate_or_open_existing_entry(project_path, &pane, cx)
-                {
-                    Ok(existing)
-                } else {
-                    Ok(this.add_item(item, cx))
-                }
+                Ok(this.open_item_in_pane(item, &pane, cx))
             })
-        }))
+        })
     }
 
-    fn activate_or_open_existing_entry(
-        &mut self,
-        project_path: ProjectPath,
-        pane: &ViewHandle<Pane>,
-        cx: &mut ViewContext<Self>,
-    ) -> Option<Box<dyn ItemViewHandle>> {
-        // If the pane contains a view for this file, then activate
-        // that item view.
-        if let Some(existing_item_view) =
-            pane.update(cx, |pane, cx| pane.activate_entry(project_path.clone(), cx))
-        {
-            return Some(existing_item_view);
-        }
-
-        // Otherwise, if this file is already open somewhere in the workspace,
-        // then add another view for it.
-        let settings = self.settings.clone();
-        let mut view_for_existing_item = None;
-        self.items.retain(|item| {
-            if let Some(item) = item.upgrade(cx) {
-                if view_for_existing_item.is_none()
-                    && item
-                        .project_path(cx)
-                        .map_or(false, |item_project_path| item_project_path == project_path)
-                {
-                    view_for_existing_item =
-                        Some(item.add_view(cx.window_id(), settings.clone(), cx.as_mut()));
-                }
-                true
-            } else {
-                false
-            }
-        });
-        if let Some(view) = view_for_existing_item {
-            pane.add_item_view(view.boxed_clone(), cx.as_mut());
-            Some(view)
-        } else {
-            None
-        }
+    fn item_for_path(&self, path: &ProjectPath, cx: &AppContext) -> Option<Box<dyn ItemHandle>> {
+        self.items
+            .iter()
+            .filter_map(|i| i.upgrade(cx))
+            .find(|i| i.project_path(cx).as_ref() == Some(path))
     }
 
     pub fn active_item(&self, cx: &AppContext) -> Option<Box<dyn ItemViewHandle>> {
@@ -908,19 +893,28 @@ impl Workspace {
         pane
     }
 
-    pub fn add_item<T>(
+    pub fn open_item<T>(
+        &mut self,
+        item_handle: T,
+        cx: &mut ViewContext<Self>,
+    ) -> Box<dyn ItemViewHandle>
+    where
+        T: 'static + ItemHandle,
+    {
+        self.open_item_in_pane(item_handle, &self.active_pane().clone(), cx)
+    }
+
+    pub fn open_item_in_pane<T>(
         &mut self,
         item_handle: T,
+        pane: &ViewHandle<Pane>,
         cx: &mut ViewContext<Self>,
     ) -> Box<dyn ItemViewHandle>
     where
-        T: ItemHandle,
+        T: 'static + ItemHandle,
     {
-        let view = item_handle.add_view(cx.window_id(), self.settings.clone(), cx);
-        self.items.push(item_handle.downgrade());
-        self.active_pane()
-            .add_item_view(view.boxed_clone(), cx.as_mut());
-        view
+        self.items.insert(item_handle.downgrade());
+        pane.update(cx, |pane, cx| pane.open_item(item_handle, cx))
     }
 
     fn activate_pane(&mut self, pane: ViewHandle<Pane>, cx: &mut ViewContext<Self>) {
@@ -965,7 +959,7 @@ impl Workspace {
         self.activate_pane(new_pane.clone(), cx);
         if let Some(item) = pane.read(cx).active_item() {
             if let Some(clone) = item.clone_on_split(cx.as_mut()) {
-                new_pane.add_item_view(clone, cx.as_mut());
+                new_pane.update(cx, |new_pane, cx| new_pane.add_item_view(clone, cx));
             }
         }
         self.center

crates/zed/src/main.rs 🔗

@@ -51,11 +51,11 @@ fn main() {
         let http = http::client();
         let client = client::Client::new(http.clone());
         let user_store = cx.add_model(|cx| UserStore::new(client.clone(), http.clone(), cx));
-        let mut entry_openers = Vec::new();
+        let mut path_openers = Vec::new();
 
         client::init(client.clone(), cx);
         workspace::init(cx);
-        editor::init(cx, &mut entry_openers);
+        editor::init(cx, &mut path_openers);
         go_to_line::init(cx);
         file_finder::init(cx);
         chat_panel::init(cx);
@@ -72,7 +72,7 @@ fn main() {
             client,
             user_store,
             fs: Arc::new(RealFs),
-            entry_openers: Arc::from(entry_openers),
+            path_openers: Arc::from(path_openers),
             build_window_options: &build_window_options,
             build_workspace: &build_workspace,
         });

crates/zed/src/test.rs 🔗

@@ -16,8 +16,8 @@ fn init_logger() {
 }
 
 pub fn test_app_state(cx: &mut MutableAppContext) -> Arc<AppState> {
-    let mut entry_openers = Vec::new();
-    editor::init(cx, &mut entry_openers);
+    let mut path_openers = Vec::new();
+    editor::init(cx, &mut path_openers);
     let (settings_tx, settings) = watch::channel_with(build_settings(cx));
     let themes = ThemeRegistry::new(Assets, cx.font_cache().clone());
     let http = FakeHttpClient::with_404_response();
@@ -41,7 +41,7 @@ pub fn test_app_state(cx: &mut MutableAppContext) -> Arc<AppState> {
         client,
         user_store,
         fs: Arc::new(FakeFs::new()),
-        entry_openers: Arc::from(entry_openers),
+        path_openers: Arc::from(path_openers),
         build_window_options: &build_window_options,
         build_workspace: &build_workspace,
     })

crates/zed/src/zed.rs 🔗

@@ -62,7 +62,7 @@ pub fn build_workspace(
         settings: app_state.settings.clone(),
         user_store: app_state.user_store.clone(),
         channel_list: app_state.channel_list.clone(),
-        entry_openers: app_state.entry_openers.clone(),
+        path_openers: app_state.path_openers.clone(),
     };
     let mut workspace = Workspace::new(&workspace_params, cx);
     let project = workspace.project().clone();
@@ -265,7 +265,6 @@ mod tests {
         // Open the first entry
         let entry_1 = workspace
             .update(&mut cx, |w, cx| w.open_path(file1.clone(), cx))
-            .unwrap()
             .await
             .unwrap();
         cx.read(|cx| {
@@ -274,13 +273,12 @@ mod tests {
                 pane.active_item().unwrap().project_path(cx),
                 Some(file1.clone())
             );
-            assert_eq!(pane.items().len(), 1);
+            assert_eq!(pane.item_views().len(), 1);
         });
 
         // Open the second entry
         workspace
             .update(&mut cx, |w, cx| w.open_path(file2.clone(), cx))
-            .unwrap()
             .await
             .unwrap();
         cx.read(|cx| {
@@ -289,12 +287,12 @@ mod tests {
                 pane.active_item().unwrap().project_path(cx),
                 Some(file2.clone())
             );
-            assert_eq!(pane.items().len(), 2);
+            assert_eq!(pane.item_views().len(), 2);
         });
 
         // Open the first entry again. The existing pane item is activated.
         let entry_1b = workspace
-            .update(&mut cx, |w, cx| w.open_path(file1.clone(), cx).unwrap())
+            .update(&mut cx, |w, cx| w.open_path(file1.clone(), cx))
             .await
             .unwrap();
         assert_eq!(entry_1.id(), entry_1b.id());
@@ -305,14 +303,14 @@ mod tests {
                 pane.active_item().unwrap().project_path(cx),
                 Some(file1.clone())
             );
-            assert_eq!(pane.items().len(), 2);
+            assert_eq!(pane.item_views().len(), 2);
         });
 
         // Split the pane with the first entry, then open the second entry again.
         workspace
             .update(&mut cx, |w, cx| {
                 w.split_pane(w.active_pane().clone(), SplitDirection::Right, cx);
-                w.open_path(file2.clone(), cx).unwrap()
+                w.open_path(file2.clone(), cx)
             })
             .await
             .unwrap();
@@ -331,8 +329,8 @@ mod tests {
         // Open the third entry twice concurrently. Only one pane item is added.
         let (t1, t2) = workspace.update(&mut cx, |w, cx| {
             (
-                w.open_path(file3.clone(), cx).unwrap(),
-                w.open_path(file3.clone(), cx).unwrap(),
+                w.open_path(file3.clone(), cx),
+                w.open_path(file3.clone(), cx),
             )
         });
         t1.await.unwrap();
@@ -344,7 +342,7 @@ mod tests {
                 Some(file3.clone())
             );
             let pane_entries = pane
-                .items()
+                .item_views()
                 .iter()
                 .map(|i| i.project_path(cx).unwrap())
                 .collect::<Vec<_>>();
@@ -561,15 +559,13 @@ mod tests {
         workspace
             .update(&mut cx, |workspace, cx| {
                 workspace.split_pane(workspace.active_pane().clone(), SplitDirection::Right, cx);
-                workspace
-                    .open_path(
-                        ProjectPath {
-                            worktree_id: worktree.read(cx).id(),
-                            path: Path::new("the-new-name.rs").into(),
-                        },
-                        cx,
-                    )
-                    .unwrap()
+                workspace.open_path(
+                    ProjectPath {
+                        worktree_id: worktree.read(cx).id(),
+                        path: Path::new("the-new-name.rs").into(),
+                    },
+                    cx,
+                )
             })
             .await
             .unwrap();
@@ -667,7 +663,6 @@ mod tests {
 
         workspace
             .update(&mut cx, |w, cx| w.open_path(file1.clone(), cx))
-            .unwrap()
             .await
             .unwrap();
         cx.read(|cx| {