Move history navigation logic to `Workspace`

Antonio Scandurra and Nathan Sobo created

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

Change summary

crates/collab/src/tests/integration_tests.rs |   6 
crates/editor/src/editor.rs                  |   4 
crates/workspace/src/pane.rs                 | 362 ++++++---------------
crates/workspace/src/shared_screen.rs        |   2 
crates/workspace/src/toolbar.rs              |  10 
crates/workspace/src/workspace.rs            | 126 +++++++
crates/zed/src/menus.rs                      |   4 
crates/zed/src/zed.rs                        |  72 ++-
8 files changed, 293 insertions(+), 293 deletions(-)

Detailed changes

crates/collab/src/tests/integration_tests.rs 🔗

@@ -6608,7 +6608,7 @@ async fn test_basic_following(
     // When client A navigates back and forth, client B does so as well.
     workspace_a
         .update(cx_a, |workspace, cx| {
-            workspace::Pane::go_back(workspace, None, cx)
+            workspace.go_back(workspace.active_pane().downgrade(), cx)
         })
         .await
         .unwrap();
@@ -6619,7 +6619,7 @@ async fn test_basic_following(
 
     workspace_a
         .update(cx_a, |workspace, cx| {
-            workspace::Pane::go_back(workspace, None, cx)
+            workspace.go_back(workspace.active_pane().downgrade(), cx)
         })
         .await
         .unwrap();
@@ -6630,7 +6630,7 @@ async fn test_basic_following(
 
     workspace_a
         .update(cx_a, |workspace, cx| {
-            workspace::Pane::go_forward(workspace, None, cx)
+            workspace.go_forward(workspace.active_pane().downgrade(), cx)
         })
         .await
         .unwrap();

crates/editor/src/editor.rs 🔗

@@ -4931,12 +4931,12 @@ impl Editor {
     }
 
     fn push_to_nav_history(
-        &self,
+        &mut self,
         cursor_anchor: Anchor,
         new_position: Option<Point>,
         cx: &mut ViewContext<Self>,
     ) {
-        if let Some(nav_history) = &self.nav_history {
+        if let Some(nav_history) = self.nav_history.as_mut() {
             let buffer = self.buffer.read(cx).read(cx);
             let cursor_position = cursor_anchor.to_point(&buffer);
             let scroll_state = self.scroll_manager.anchor();

crates/workspace/src/pane.rs 🔗

@@ -39,7 +39,6 @@ use std::{
     },
 };
 use theme::{Theme, ThemeSettings};
-use util::ResultExt;
 
 #[derive(Clone, Deserialize, PartialEq)]
 pub struct ActivateItem(pub usize);
@@ -74,6 +73,8 @@ actions!(
         CloseItemsToTheLeft,
         CloseItemsToTheRight,
         CloseAllItems,
+        GoBack,
+        GoForward,
         ReopenClosedItem,
         SplitLeft,
         SplitUp,
@@ -82,19 +83,7 @@ actions!(
     ]
 );
 
-#[derive(Clone, Deserialize, PartialEq)]
-pub struct GoBack {
-    #[serde(skip_deserializing)]
-    pub pane: Option<WeakViewHandle<Pane>>,
-}
-
-#[derive(Clone, Deserialize, PartialEq)]
-pub struct GoForward {
-    #[serde(skip_deserializing)]
-    pub pane: Option<WeakViewHandle<Pane>>,
-}
-
-impl_actions!(pane, [GoBack, GoForward, ActivateItem]);
+impl_actions!(pane, [ActivateItem]);
 
 const MAX_NAVIGATION_HISTORY_LEN: usize = 1024;
 
@@ -124,15 +113,6 @@ pub fn init(cx: &mut AppContext) {
     cx.add_action(|pane: &mut Pane, _: &SplitUp, cx| pane.split(SplitDirection::Up, cx));
     cx.add_action(|pane: &mut Pane, _: &SplitRight, cx| pane.split(SplitDirection::Right, cx));
     cx.add_action(|pane: &mut Pane, _: &SplitDown, cx| pane.split(SplitDirection::Down, cx));
-    cx.add_action(|workspace: &mut Workspace, _: &ReopenClosedItem, cx| {
-        Pane::reopen_closed_item(workspace, cx).detach();
-    });
-    cx.add_action(|workspace: &mut Workspace, action: &GoBack, cx| {
-        Pane::go_back(workspace, action.pane.clone(), cx).detach();
-    });
-    cx.add_action(|workspace: &mut Workspace, action: &GoForward, cx| {
-        Pane::go_forward(workspace, action.pane.clone(), cx).detach();
-    });
 }
 
 #[derive(Debug)]
@@ -155,7 +135,7 @@ pub struct Pane {
     active_item_index: usize,
     last_focused_view_by_item: HashMap<usize, AnyWeakViewHandle>,
     autoscroll: bool,
-    nav_history: Rc<RefCell<NavHistory>>,
+    nav_history: NavHistory,
     toolbar: ViewHandle<Toolbar>,
     tab_bar_context_menu: TabBarContextMenu,
     tab_context_menu: ViewHandle<ContextMenu>,
@@ -165,18 +145,18 @@ pub struct Pane {
     has_focus: bool,
     can_drop: Rc<dyn Fn(&DragAndDrop<Workspace>, &WindowContext) -> bool>,
     can_split: bool,
-    can_navigate: bool,
     render_tab_bar_buttons: Rc<dyn Fn(&mut Pane, &mut ViewContext<Pane>) -> AnyElement<Pane>>,
 }
 
 pub struct ItemNavHistory {
-    history: Rc<RefCell<NavHistory>>,
+    history: NavHistory,
     item: Rc<dyn WeakItemHandle>,
 }
 
-pub struct PaneNavHistory(Rc<RefCell<NavHistory>>);
+#[derive(Clone)]
+pub struct NavHistory(Rc<RefCell<NavHistoryState>>);
 
-struct NavHistory {
+struct NavHistoryState {
     mode: NavigationMode,
     backward_stack: VecDeque<NavigationEntry>,
     forward_stack: VecDeque<NavigationEntry>,
@@ -187,7 +167,7 @@ struct NavHistory {
 }
 
 #[derive(Copy, Clone)]
-enum NavigationMode {
+pub enum NavigationMode {
     Normal,
     GoingBack,
     GoingForward,
@@ -262,7 +242,7 @@ impl Pane {
             active_item_index: 0,
             last_focused_view_by_item: Default::default(),
             autoscroll: false,
-            nav_history: Rc::new(RefCell::new(NavHistory {
+            nav_history: NavHistory(Rc::new(RefCell::new(NavHistoryState {
                 mode: NavigationMode::Normal,
                 backward_stack: Default::default(),
                 forward_stack: Default::default(),
@@ -270,7 +250,7 @@ impl Pane {
                 paths_by_item: Default::default(),
                 pane: handle.clone(),
                 next_timestamp,
-            })),
+            }))),
             toolbar: cx.add_view(|_| Toolbar::new(handle)),
             tab_bar_context_menu: TabBarContextMenu {
                 kind: TabBarContextMenuKind::New,
@@ -283,7 +263,6 @@ impl Pane {
             has_focus: false,
             can_drop: Rc::new(|_, _| true),
             can_split: true,
-            can_navigate: true,
             render_tab_bar_buttons: Rc::new(|pane, cx| {
                 Flex::row()
                     // New menu
@@ -339,6 +318,10 @@ impl Pane {
         self.has_focus
     }
 
+    pub fn active_item_index(&self) -> usize {
+        self.active_item_index
+    }
+
     pub fn on_can_drop<F>(&mut self, can_drop: F)
     where
         F: 'static + Fn(&DragAndDrop<Workspace>, &WindowContext) -> bool,
@@ -352,7 +335,6 @@ impl Pane {
     }
 
     pub fn set_can_navigate(&mut self, can_navigate: bool, cx: &mut ViewContext<Self>) {
-        self.can_navigate = can_navigate;
         self.toolbar.update(cx, |toolbar, cx| {
             toolbar.set_can_navigate(can_navigate, cx);
         });
@@ -374,164 +356,34 @@ impl Pane {
         }
     }
 
-    pub fn nav_history(&self) -> PaneNavHistory {
-        PaneNavHistory(self.nav_history.clone())
-    }
-
-    pub fn go_back(
-        workspace: &mut Workspace,
-        pane: Option<WeakViewHandle<Pane>>,
-        cx: &mut ViewContext<Workspace>,
-    ) -> Task<Result<()>> {
-        Self::navigate_history(
-            workspace,
-            pane.unwrap_or_else(|| workspace.active_pane().downgrade()),
-            NavigationMode::GoingBack,
-            cx,
-        )
-    }
-
-    pub fn go_forward(
-        workspace: &mut Workspace,
-        pane: Option<WeakViewHandle<Pane>>,
-        cx: &mut ViewContext<Workspace>,
-    ) -> Task<Result<()>> {
-        Self::navigate_history(
-            workspace,
-            pane.unwrap_or_else(|| workspace.active_pane().downgrade()),
-            NavigationMode::GoingForward,
-            cx,
-        )
+    pub fn nav_history(&self) -> &NavHistory {
+        &self.nav_history
     }
 
-    pub fn reopen_closed_item(
-        workspace: &mut Workspace,
-        cx: &mut ViewContext<Workspace>,
-    ) -> Task<Result<()>> {
-        Self::navigate_history(
-            workspace,
-            workspace.active_pane().downgrade(),
-            NavigationMode::ReopeningClosedItem,
-            cx,
-        )
+    pub fn nav_history_mut(&mut self) -> &mut NavHistory {
+        &mut self.nav_history
     }
 
     pub fn disable_history(&mut self) {
-        self.nav_history.borrow_mut().disable();
+        self.nav_history.disable();
     }
 
     pub fn enable_history(&mut self) {
-        self.nav_history.borrow_mut().enable();
+        self.nav_history.enable();
     }
 
     pub fn can_navigate_backward(&self) -> bool {
-        !self.nav_history.borrow().backward_stack.is_empty()
+        !self.nav_history.0.borrow().backward_stack.is_empty()
     }
 
     pub fn can_navigate_forward(&self) -> bool {
-        !self.nav_history.borrow().forward_stack.is_empty()
+        !self.nav_history.0.borrow().forward_stack.is_empty()
     }
 
     fn history_updated(&mut self, cx: &mut ViewContext<Self>) {
         self.toolbar.update(cx, |_, cx| cx.notify());
     }
 
-    fn navigate_history(
-        workspace: &mut Workspace,
-        pane: WeakViewHandle<Pane>,
-        mode: NavigationMode,
-        cx: &mut ViewContext<Workspace>,
-    ) -> Task<Result<()>> {
-        let to_load = if let Some(pane) = pane.upgrade(cx) {
-            if !pane.read(cx).can_navigate {
-                return Task::ready(Ok(()));
-            }
-
-            cx.focus(&pane);
-
-            pane.update(cx, |pane, cx| {
-                loop {
-                    // Retrieve the weak item handle from the history.
-                    let entry = pane.nav_history.borrow_mut().pop(mode, cx)?;
-
-                    // If the item is still present in this pane, then activate it.
-                    if let Some(index) = entry
-                        .item
-                        .upgrade(cx)
-                        .and_then(|v| pane.index_for_item(v.as_ref()))
-                    {
-                        let prev_active_item_index = pane.active_item_index;
-                        pane.nav_history.borrow_mut().set_mode(mode);
-                        pane.activate_item(index, true, true, cx);
-                        pane.nav_history
-                            .borrow_mut()
-                            .set_mode(NavigationMode::Normal);
-
-                        let mut navigated = prev_active_item_index != pane.active_item_index;
-                        if let Some(data) = entry.data {
-                            navigated |= pane.active_item()?.navigate(data, cx);
-                        }
-
-                        if navigated {
-                            break None;
-                        }
-                    }
-                    // If the item is no longer present in this pane, then retrieve its
-                    // project path in order to reopen it.
-                    else {
-                        break pane
-                            .nav_history
-                            .borrow()
-                            .paths_by_item
-                            .get(&entry.item.id())
-                            .cloned()
-                            .map(|(project_path, _)| (project_path, entry));
-                    }
-                }
-            })
-        } else {
-            None
-        };
-
-        if let Some((project_path, entry)) = to_load {
-            // If the item was no longer present, then load it again from its previous path.
-            let task = workspace.load_path(project_path, cx);
-            cx.spawn(|workspace, mut cx| async move {
-                let task = task.await;
-                let mut navigated = false;
-                if let Some((project_entry_id, build_item)) = task.log_err() {
-                    let prev_active_item_id = pane.update(&mut cx, |pane, _| {
-                        pane.nav_history.borrow_mut().set_mode(mode);
-                        pane.active_item().map(|p| p.id())
-                    })?;
-
-                    pane.update(&mut cx, |pane, cx| {
-                        let item = pane.open_item(project_entry_id, true, cx, build_item);
-                        navigated |= Some(item.id()) != prev_active_item_id;
-                        pane.nav_history
-                            .borrow_mut()
-                            .set_mode(NavigationMode::Normal);
-                        if let Some(data) = entry.data {
-                            navigated |= item.navigate(data, cx);
-                        }
-                    })?;
-                }
-
-                if !navigated {
-                    workspace
-                        .update(&mut cx, |workspace, cx| {
-                            Self::navigate_history(workspace, pane, mode, cx)
-                        })?
-                        .await?;
-                }
-
-                Ok(())
-            })
-        } else {
-            Task::ready(Ok(()))
-        }
-    }
-
     pub(crate) fn open_item(
         &mut self,
         project_entry_id: ProjectEntryId,
@@ -573,6 +425,7 @@ impl Pane {
                 if let Some(project_path) = project.path_for_entry(entry_id, cx) {
                     let abs_path = project.absolute_path(&project_path, cx);
                     self.nav_history
+                        .0
                         .borrow_mut()
                         .paths_by_item
                         .insert(item.id(), (project_path, abs_path));
@@ -713,7 +566,7 @@ impl Pane {
         if index < self.items.len() {
             let prev_active_item_ix = mem::replace(&mut self.active_item_index, index);
             if prev_active_item_ix != self.active_item_index
-                || matches!(self.nav_history.borrow().mode, GoingBack | GoingForward)
+                || matches!(self.nav_history.mode(), GoingBack | GoingForward)
             {
                 if let Some(prev_item) = self.items.get(prev_active_item_ix) {
                     prev_item.deactivated(cx);
@@ -981,27 +834,26 @@ impl Pane {
             self.active_item_index -= 1;
         }
 
-        self.nav_history
-            .borrow_mut()
-            .set_mode(NavigationMode::ClosingItem);
+        self.nav_history.set_mode(NavigationMode::ClosingItem);
         item.deactivated(cx);
-        self.nav_history
-            .borrow_mut()
-            .set_mode(NavigationMode::Normal);
+        self.nav_history.set_mode(NavigationMode::Normal);
 
         if let Some(path) = item.project_path(cx) {
             let abs_path = self
                 .nav_history
+                .0
                 .borrow()
                 .paths_by_item
                 .get(&item.id())
                 .and_then(|(_, abs_path)| abs_path.clone());
             self.nav_history
+                .0
                 .borrow_mut()
                 .paths_by_item
                 .insert(item.id(), (path, abs_path));
         } else {
             self.nav_history
+                .0
                 .borrow_mut()
                 .paths_by_item
                 .remove(&item.id());
@@ -1249,7 +1101,7 @@ impl Pane {
         })?;
 
         self.remove_item(item_index_to_delete, false, cx);
-        self.nav_history.borrow_mut().remove_item(item_id);
+        self.nav_history.remove_item(item_id);
 
         Some(())
     }
@@ -1719,7 +1571,7 @@ impl View for Pane {
                     let pane = cx.weak_handle();
                     cx.window_context().defer(move |cx| {
                         workspace.update(cx, |workspace, cx| {
-                            Pane::go_back(workspace, Some(pane), cx).detach_and_log_err(cx)
+                            workspace.go_back(pane, cx).detach_and_log_err(cx)
                         })
                     })
                 }
@@ -1731,7 +1583,7 @@ impl View for Pane {
                     let pane = cx.weak_handle();
                     cx.window_context().defer(move |cx| {
                         workspace.update(cx, |workspace, cx| {
-                            Pane::go_forward(workspace, Some(pane), cx).detach_and_log_err(cx)
+                            workspace.go_forward(pane, cx).detach_and_log_err(cx)
                         })
                     })
                 }
@@ -1786,42 +1638,69 @@ impl View for Pane {
 }
 
 impl ItemNavHistory {
-    pub fn push<D: 'static + Any>(&self, data: Option<D>, cx: &mut WindowContext) {
-        self.history.borrow_mut().push(data, self.item.clone(), cx);
+    pub fn push<D: 'static + Any>(&mut self, data: Option<D>, cx: &mut WindowContext) {
+        self.history.push(data, self.item.clone(), cx);
     }
 
-    pub fn pop_backward(&self, cx: &mut WindowContext) -> Option<NavigationEntry> {
-        self.history.borrow_mut().pop(NavigationMode::GoingBack, cx)
+    pub fn pop_backward(&mut self, cx: &mut WindowContext) -> Option<NavigationEntry> {
+        self.history.pop(NavigationMode::GoingBack, cx)
     }
 
-    pub fn pop_forward(&self, cx: &mut WindowContext) -> Option<NavigationEntry> {
-        self.history
-            .borrow_mut()
-            .pop(NavigationMode::GoingForward, cx)
+    pub fn pop_forward(&mut self, cx: &mut WindowContext) -> Option<NavigationEntry> {
+        self.history.pop(NavigationMode::GoingForward, cx)
     }
 }
 
 impl NavHistory {
-    fn set_mode(&mut self, mode: NavigationMode) {
-        self.mode = mode;
+    pub fn for_each_entry(
+        &self,
+        cx: &AppContext,
+        mut f: impl FnMut(&NavigationEntry, (ProjectPath, Option<PathBuf>)),
+    ) {
+        let borrowed_history = self.0.borrow();
+        borrowed_history
+            .forward_stack
+            .iter()
+            .chain(borrowed_history.backward_stack.iter())
+            .chain(borrowed_history.closed_stack.iter())
+            .for_each(|entry| {
+                if let Some(project_and_abs_path) =
+                    borrowed_history.paths_by_item.get(&entry.item.id())
+                {
+                    f(entry, project_and_abs_path.clone());
+                } else if let Some(item) = entry.item.upgrade(cx) {
+                    if let Some(path) = item.project_path(cx) {
+                        f(entry, (path, None));
+                    }
+                }
+            })
+    }
+
+    pub fn set_mode(&mut self, mode: NavigationMode) {
+        self.0.borrow_mut().mode = mode;
     }
 
-    fn disable(&mut self) {
-        self.mode = NavigationMode::Disabled;
+    pub fn mode(&self) -> NavigationMode {
+        self.0.borrow().mode
     }
 
-    fn enable(&mut self) {
-        self.mode = NavigationMode::Normal;
+    pub fn disable(&mut self) {
+        self.0.borrow_mut().mode = NavigationMode::Disabled;
     }
 
-    fn pop(&mut self, mode: NavigationMode, cx: &mut WindowContext) -> Option<NavigationEntry> {
+    pub fn enable(&mut self) {
+        self.0.borrow_mut().mode = NavigationMode::Normal;
+    }
+
+    pub fn pop(&mut self, mode: NavigationMode, cx: &mut WindowContext) -> Option<NavigationEntry> {
+        let mut state = self.0.borrow_mut();
         let entry = match mode {
             NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => {
                 return None
             }
-            NavigationMode::GoingBack => &mut self.backward_stack,
-            NavigationMode::GoingForward => &mut self.forward_stack,
-            NavigationMode::ReopeningClosedItem => &mut self.closed_stack,
+            NavigationMode::GoingBack => &mut state.backward_stack,
+            NavigationMode::GoingForward => &mut state.forward_stack,
+            NavigationMode::ReopeningClosedItem => &mut state.closed_stack,
         }
         .pop_back();
         if entry.is_some() {
@@ -1830,100 +1709,85 @@ impl NavHistory {
         entry
     }
 
-    fn push<D: 'static + Any>(
+    pub fn push<D: 'static + Any>(
         &mut self,
         data: Option<D>,
         item: Rc<dyn WeakItemHandle>,
         cx: &mut WindowContext,
     ) {
-        match self.mode {
+        let state = &mut *self.0.borrow_mut();
+        match state.mode {
             NavigationMode::Disabled => {}
             NavigationMode::Normal | NavigationMode::ReopeningClosedItem => {
-                if self.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
-                    self.backward_stack.pop_front();
+                if state.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
+                    state.backward_stack.pop_front();
                 }
-                self.backward_stack.push_back(NavigationEntry {
+                state.backward_stack.push_back(NavigationEntry {
                     item,
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    timestamp: self.next_timestamp.fetch_add(1, Ordering::SeqCst),
+                    timestamp: state.next_timestamp.fetch_add(1, Ordering::SeqCst),
                 });
-                self.forward_stack.clear();
+                state.forward_stack.clear();
             }
             NavigationMode::GoingBack => {
-                if self.forward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
-                    self.forward_stack.pop_front();
+                if state.forward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
+                    state.forward_stack.pop_front();
                 }
-                self.forward_stack.push_back(NavigationEntry {
+                state.forward_stack.push_back(NavigationEntry {
                     item,
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    timestamp: self.next_timestamp.fetch_add(1, Ordering::SeqCst),
+                    timestamp: state.next_timestamp.fetch_add(1, Ordering::SeqCst),
                 });
             }
             NavigationMode::GoingForward => {
-                if self.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
-                    self.backward_stack.pop_front();
+                if state.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
+                    state.backward_stack.pop_front();
                 }
-                self.backward_stack.push_back(NavigationEntry {
+                state.backward_stack.push_back(NavigationEntry {
                     item,
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    timestamp: self.next_timestamp.fetch_add(1, Ordering::SeqCst),
+                    timestamp: state.next_timestamp.fetch_add(1, Ordering::SeqCst),
                 });
             }
             NavigationMode::ClosingItem => {
-                if self.closed_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
-                    self.closed_stack.pop_front();
+                if state.closed_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
+                    state.closed_stack.pop_front();
                 }
-                self.closed_stack.push_back(NavigationEntry {
+                state.closed_stack.push_back(NavigationEntry {
                     item,
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
-                    timestamp: self.next_timestamp.fetch_add(1, Ordering::SeqCst),
+                    timestamp: state.next_timestamp.fetch_add(1, Ordering::SeqCst),
                 });
             }
         }
         self.did_update(cx);
     }
 
-    fn did_update(&self, cx: &mut WindowContext) {
-        if let Some(pane) = self.pane.upgrade(cx) {
+    pub fn did_update(&self, cx: &mut WindowContext) {
+        let state = self.0.borrow();
+        if let Some(pane) = state.pane.upgrade(cx) {
             cx.defer(move |cx| {
                 pane.update(cx, |pane, cx| pane.history_updated(cx));
             });
         }
     }
 
-    fn remove_item(&mut self, item_id: usize) {
-        self.paths_by_item.remove(&item_id);
-        self.backward_stack
+    pub fn remove_item(&mut self, item_id: usize) {
+        let mut state = self.0.borrow_mut();
+        state.paths_by_item.remove(&item_id);
+        state
+            .backward_stack
+            .retain(|entry| entry.item.id() != item_id);
+        state
+            .forward_stack
             .retain(|entry| entry.item.id() != item_id);
-        self.forward_stack
+        state
+            .closed_stack
             .retain(|entry| entry.item.id() != item_id);
-        self.closed_stack.retain(|entry| entry.item.id() != item_id);
     }
-}
 
-impl PaneNavHistory {
-    pub fn for_each_entry(
-        &self,
-        cx: &AppContext,
-        mut f: impl FnMut(&NavigationEntry, (ProjectPath, Option<PathBuf>)),
-    ) {
-        let borrowed_history = self.0.borrow();
-        borrowed_history
-            .forward_stack
-            .iter()
-            .chain(borrowed_history.backward_stack.iter())
-            .chain(borrowed_history.closed_stack.iter())
-            .for_each(|entry| {
-                if let Some(project_and_abs_path) =
-                    borrowed_history.paths_by_item.get(&entry.item.id())
-                {
-                    f(entry, project_and_abs_path.clone());
-                } else if let Some(item) = entry.item.upgrade(cx) {
-                    if let Some(path) = item.project_path(cx) {
-                        f(entry, (path, None));
-                    }
-                }
-            })
+    pub fn path_for_item(&self, item_id: usize) -> Option<(ProjectPath, Option<PathBuf>)> {
+        self.0.borrow().paths_by_item.get(&item_id).cloned()
     }
 }
 

crates/workspace/src/shared_screen.rs 🔗

@@ -99,7 +99,7 @@ impl Item for SharedScreen {
         Some(format!("{}'s screen", self.user.github_login).into())
     }
     fn deactivated(&mut self, cx: &mut ViewContext<Self>) {
-        if let Some(nav_history) = self.nav_history.as_ref() {
+        if let Some(nav_history) = self.nav_history.as_mut() {
             nav_history.push::<()>(None, cx);
         }
     }

crates/workspace/src/toolbar.rs 🔗

@@ -153,14 +153,13 @@ impl View for Toolbar {
                             let pane = pane.clone();
                             cx.window_context().defer(move |cx| {
                                 workspace.update(cx, |workspace, cx| {
-                                    Pane::go_back(workspace, Some(pane.clone()), cx)
-                                        .detach_and_log_err(cx);
+                                    workspace.go_back(pane.clone(), cx).detach_and_log_err(cx);
                                 });
                             })
                         }
                     }
                 },
-                super::GoBack { pane: None },
+                super::GoBack,
                 "Go Back",
                 cx,
             ));
@@ -182,14 +181,15 @@ impl View for Toolbar {
                             let pane = pane.clone();
                             cx.window_context().defer(move |cx| {
                                 workspace.update(cx, |workspace, cx| {
-                                    Pane::go_forward(workspace, Some(pane.clone()), cx)
+                                    workspace
+                                        .go_forward(pane.clone(), cx)
                                         .detach_and_log_err(cx);
                                 });
                             });
                         }
                     }
                 },
-                super::GoForward { pane: None },
+                super::GoForward,
                 "Go Forward",
                 cx,
             ));

crates/workspace/src/workspace.rs 🔗

@@ -278,6 +278,19 @@ pub fn init(app_state: Arc<AppState>, cx: &mut AppContext) {
         workspace.toggle_dock(DockPosition::Bottom, action.focus, cx);
     });
     cx.add_action(Workspace::activate_pane_at_index);
+    cx.add_action(|workspace: &mut Workspace, _: &ReopenClosedItem, cx| {
+        workspace.reopen_closed_item(cx).detach();
+    });
+    cx.add_action(|workspace: &mut Workspace, _: &GoBack, cx| {
+        workspace
+            .go_back(workspace.active_pane().downgrade(), cx)
+            .detach();
+    });
+    cx.add_action(|workspace: &mut Workspace, _: &GoForward, cx| {
+        workspace
+            .go_forward(workspace.active_pane().downgrade(), cx)
+            .detach();
+    });
 
     cx.add_action(|_: &mut Workspace, _: &install_cli::Install, cx| {
         cx.spawn(|workspace, mut cx| async move {
@@ -1000,6 +1013,115 @@ impl Workspace {
             .collect()
     }
 
+    fn navigate_history(
+        &mut self,
+        pane: WeakViewHandle<Pane>,
+        mode: NavigationMode,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<Result<()>> {
+        let to_load = if let Some(pane) = pane.upgrade(cx) {
+            cx.focus(&pane);
+
+            pane.update(cx, |pane, cx| {
+                loop {
+                    // Retrieve the weak item handle from the history.
+                    let entry = pane.nav_history_mut().pop(mode, cx)?;
+
+                    // If the item is still present in this pane, then activate it.
+                    if let Some(index) = entry
+                        .item
+                        .upgrade(cx)
+                        .and_then(|v| pane.index_for_item(v.as_ref()))
+                    {
+                        let prev_active_item_index = pane.active_item_index();
+                        pane.nav_history_mut().set_mode(mode);
+                        pane.activate_item(index, true, true, cx);
+                        pane.nav_history_mut().set_mode(NavigationMode::Normal);
+
+                        let mut navigated = prev_active_item_index != pane.active_item_index();
+                        if let Some(data) = entry.data {
+                            navigated |= pane.active_item()?.navigate(data, cx);
+                        }
+
+                        if navigated {
+                            break None;
+                        }
+                    }
+                    // If the item is no longer present in this pane, then retrieve its
+                    // project path in order to reopen it.
+                    else {
+                        break pane
+                            .nav_history()
+                            .path_for_item(entry.item.id())
+                            .map(|(project_path, _)| (project_path, entry));
+                    }
+                }
+            })
+        } else {
+            None
+        };
+
+        if let Some((project_path, entry)) = to_load {
+            // If the item was no longer present, then load it again from its previous path.
+            let task = self.load_path(project_path, cx);
+            cx.spawn(|workspace, mut cx| async move {
+                let task = task.await;
+                let mut navigated = false;
+                if let Some((project_entry_id, build_item)) = task.log_err() {
+                    let prev_active_item_id = pane.update(&mut cx, |pane, _| {
+                        pane.nav_history_mut().set_mode(mode);
+                        pane.active_item().map(|p| p.id())
+                    })?;
+
+                    pane.update(&mut cx, |pane, cx| {
+                        let item = pane.open_item(project_entry_id, true, cx, build_item);
+                        navigated |= Some(item.id()) != prev_active_item_id;
+                        pane.nav_history_mut().set_mode(NavigationMode::Normal);
+                        if let Some(data) = entry.data {
+                            navigated |= item.navigate(data, cx);
+                        }
+                    })?;
+                }
+
+                if !navigated {
+                    workspace
+                        .update(&mut cx, |workspace, cx| {
+                            Self::navigate_history(workspace, pane, mode, cx)
+                        })?
+                        .await?;
+                }
+
+                Ok(())
+            })
+        } else {
+            Task::ready(Ok(()))
+        }
+    }
+
+    pub fn go_back(
+        &mut self,
+        pane: WeakViewHandle<Pane>,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<Result<()>> {
+        self.navigate_history(pane, NavigationMode::GoingBack, cx)
+    }
+
+    pub fn go_forward(
+        &mut self,
+        pane: WeakViewHandle<Pane>,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<Result<()>> {
+        self.navigate_history(pane, NavigationMode::GoingForward, cx)
+    }
+
+    pub fn reopen_closed_item(&mut self, cx: &mut ViewContext<Workspace>) -> Task<Result<()>> {
+        self.navigate_history(
+            self.active_pane().downgrade(),
+            NavigationMode::ReopeningClosedItem,
+            cx,
+        )
+    }
+
     pub fn client(&self) -> &Client {
         &self.app_state.client
     }
@@ -4037,9 +4159,7 @@ mod tests {
         });
 
         workspace
-            .update(cx, |workspace, cx| {
-                Pane::go_back(workspace, Some(pane.downgrade()), cx)
-            })
+            .update(cx, |workspace, cx| workspace.go_back(pane.downgrade(), cx))
             .await
             .unwrap();
 

crates/zed/src/menus.rs 🔗

@@ -120,8 +120,8 @@ pub fn menus() -> Vec<Menu<'static>> {
         Menu {
             name: "Go",
             items: vec![
-                MenuItem::action("Back", workspace::GoBack { pane: None }),
-                MenuItem::action("Forward", workspace::GoForward { pane: None }),
+                MenuItem::action("Back", workspace::GoBack),
+                MenuItem::action("Forward", workspace::GoForward),
                 MenuItem::separator(),
                 MenuItem::action("Go to File", file_finder::Toggle),
                 MenuItem::action("Go to Symbol in Project", project_symbols::Toggle),

crates/zed/src/zed.rs 🔗

@@ -663,7 +663,7 @@ mod tests {
     use util::http::FakeHttpClient;
     use workspace::{
         item::{Item, ItemHandle},
-        open_new, open_paths, pane, NewFile, Pane, SplitDirection, WorkspaceHandle,
+        open_new, open_paths, pane, NewFile, SplitDirection, WorkspaceHandle,
     };
 
     #[gpui::test]
@@ -1488,7 +1488,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1497,7 +1497,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1506,7 +1506,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1515,7 +1515,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1525,7 +1525,7 @@ mod tests {
 
         // Go back one more time and ensure we don't navigate past the first item in the history.
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1534,7 +1534,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .update(cx, |w, cx| w.go_forward(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1543,7 +1543,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .update(cx, |w, cx| w.go_forward(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1561,7 +1561,7 @@ mod tests {
         .await
         .unwrap();
         workspace
-            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .update(cx, |w, cx| w.go_forward(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1570,7 +1570,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .update(cx, |w, cx| w.go_forward(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1579,7 +1579,7 @@ mod tests {
         );
 
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1601,7 +1601,7 @@ mod tests {
             .await
             .unwrap();
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1609,7 +1609,7 @@ mod tests {
             (file1.clone(), DisplayPoint::new(10, 0), 0.)
         );
         workspace
-            .update(cx, |w, cx| Pane::go_forward(w, None, cx))
+            .update(cx, |w, cx| w.go_forward(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1653,7 +1653,7 @@ mod tests {
             })
         });
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1661,7 +1661,7 @@ mod tests {
             (file1.clone(), DisplayPoint::new(2, 0), 0.)
         );
         workspace
-            .update(cx, |w, cx| Pane::go_back(w, None, cx))
+            .update(cx, |w, cx| w.go_back(w.active_pane().downgrade(), cx))
             .await
             .unwrap();
         assert_eq!(
@@ -1766,81 +1766,97 @@ mod tests {
         // Reopen all the closed items, ensuring they are reopened in the same order
         // in which they were closed.
         workspace
-            .update(cx, Pane::reopen_closed_item)
+            .update(cx, Workspace::reopen_closed_item)
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
 
         workspace
-            .update(cx, Pane::reopen_closed_item)
+            .update(cx, Workspace::reopen_closed_item)
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file2.clone()));
 
         workspace
-            .update(cx, Pane::reopen_closed_item)
+            .update(cx, Workspace::reopen_closed_item)
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
 
         workspace
-            .update(cx, Pane::reopen_closed_item)
+            .update(cx, Workspace::reopen_closed_item)
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file1.clone()));
 
         // Reopening past the last closed item is a no-op.
         workspace
-            .update(cx, Pane::reopen_closed_item)
+            .update(cx, Workspace::reopen_closed_item)
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file1.clone()));
 
         // Reopening closed items doesn't interfere with navigation history.
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file2.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file2.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file1.clone()));
 
         workspace
-            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .update(cx, |workspace, cx| {
+                workspace.go_back(workspace.active_pane().downgrade(), cx)
+            })
             .await
             .unwrap();
         assert_eq!(active_path(&workspace, cx), Some(file1.clone()));