Merge pull request #1241 from zed-industries/reopen-closed-item

Introduce `pane::ReopenClosedItem` bound to `cmd-shift-t`

assets/keymaps/default.json

@@ -212,6 +212,7 @@
         "bindings": {
             "ctrl--": "pane::GoBack",
             "shift-ctrl-_": "pane::GoForward",
+            "cmd-shift-T": "pane::ReopenClosedItem",
             "cmd-shift-F": "project_search::ToggleFocus"
         }
     },

crates/workspace/src/pane.rs

@@ -25,6 +25,7 @@ actions!(
         ActivateNextItem,
         CloseActiveItem,
         CloseInactiveItems,
+        ReopenClosedItem,
         SplitLeft,
         SplitUp,
         SplitRight,
@@ -82,6 +83,9 @@ pub fn init(cx: &mut MutableAppContext) {
     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,
@@ -133,6 +137,7 @@ pub struct NavHistory {
     mode: NavigationMode,
     backward_stack: VecDeque<NavigationEntry>,
     forward_stack: VecDeque<NavigationEntry>,
+    closed_stack: VecDeque<NavigationEntry>,
     paths_by_item: HashMap<usize, ProjectPath>,
 }
 
@@ -141,6 +146,8 @@ enum NavigationMode {
     Normal,
     GoingBack,
     GoingForward,
+    ClosingItem,
+    ReopeningClosedItem,
     Disabled,
 }
 
@@ -200,6 +207,18 @@ impl Pane {
         )
     }
 
+    pub fn reopen_closed_item(
+        workspace: &mut Workspace,
+        cx: &mut ViewContext<Workspace>,
+    ) -> Task<()> {
+        Self::navigate_history(
+            workspace,
+            workspace.active_pane().clone(),
+            NavigationMode::ReopeningClosedItem,
+            cx,
+        )
+    }
+
     fn navigate_history(
         workspace: &mut Workspace,
         pane: ViewHandle<Pane>,
@@ -240,7 +259,7 @@ impl Pane {
                 else {
                     break pane
                         .nav_history
-                        .borrow_mut()
+                        .borrow()
                         .paths_by_item
                         .get(&entry.item.id())
                         .cloned()
@@ -256,10 +275,13 @@ impl Pane {
             cx.spawn(|workspace, mut cx| async move {
                 let task = task.await;
                 if let Some(pane) = pane.upgrade(&cx) {
+                    let mut navigated = false;
                     if let Some((project_entry_id, build_item)) = task.log_err() {
-                        pane.update(&mut cx, |pane, _| {
+                        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())
                         });
+
                         let item = workspace.update(&mut cx, |workspace, cx| {
                             Self::open_item(
                                 workspace,
@@ -270,15 +292,19 @@ impl Pane {
                                 build_item,
                             )
                         });
+
                         pane.update(&mut cx, |pane, cx| {
+                            navigated |= Some(item.id()) != prev_active_item_id;
                             pane.nav_history
                                 .borrow_mut()
                                 .set_mode(NavigationMode::Normal);
                             if let Some(data) = entry.data {
-                                item.navigate(data, cx);
+                                navigated |= item.navigate(data, cx);
                             }
                         });
-                    } else {
+                    }
+
+                    if !navigated {
                         workspace
                             .update(&mut cx, |workspace, cx| {
                                 Self::navigate_history(workspace, pane, mode, cx)
@@ -587,7 +613,15 @@ impl Pane {
                             pane.active_item_index -= 1;
                         }
 
-                        let mut nav_history = pane.nav_history.borrow_mut();
+                        pane.nav_history
+                            .borrow_mut()
+                            .set_mode(NavigationMode::ClosingItem);
+                        item.deactivated(cx);
+                        pane.nav_history
+                            .borrow_mut()
+                            .set_mode(NavigationMode::Normal);
+
+                        let mut nav_history = pane.nav_history().borrow_mut();
                         if let Some(path) = item.project_path(cx) {
                             nav_history.paths_by_item.insert(item.id(), path);
                         } else {
@@ -925,11 +959,16 @@ impl NavHistory {
         self.forward_stack.pop_back()
     }
 
+    pub fn pop_closed(&mut self) -> Option<NavigationEntry> {
+        self.closed_stack.pop_back()
+    }
+
     fn pop(&mut self, mode: NavigationMode) -> Option<NavigationEntry> {
         match mode {
-            NavigationMode::Normal | NavigationMode::Disabled => None,
+            NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => None,
             NavigationMode::GoingBack => self.pop_backward(),
             NavigationMode::GoingForward => self.pop_forward(),
+            NavigationMode::ReopeningClosedItem => self.pop_closed(),
         }
     }
 
@@ -940,7 +979,7 @@ impl NavHistory {
     pub fn push<D: 'static + Any>(&mut self, data: Option<D>, item: Rc<dyn WeakItemHandle>) {
         match self.mode {
             NavigationMode::Disabled => {}
-            NavigationMode::Normal => {
+            NavigationMode::Normal | NavigationMode::ReopeningClosedItem => {
                 if self.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
                     self.backward_stack.pop_front();
                 }
@@ -968,6 +1007,15 @@ impl NavHistory {
                     data: data.map(|data| Box::new(data) as Box<dyn Any>),
                 });
             }
+            NavigationMode::ClosingItem => {
+                if self.closed_stack.len() >= MAX_NAVIGATION_HISTORY_LEN {
+                    self.closed_stack.pop_front();
+                }
+                self.closed_stack.push_back(NavigationEntry {
+                    item,
+                    data: data.map(|data| Box::new(data) as Box<dyn Any>),
+                });
+            }
         }
     }
 }

crates/zed/src/zed.rs

@@ -1337,6 +1337,170 @@ mod tests {
         }
     }
 
+    #[gpui::test]
+    async fn test_reopening_closed_items(cx: &mut TestAppContext) {
+        let app_state = init(cx);
+        app_state
+            .fs
+            .as_fake()
+            .insert_tree(
+                "/root",
+                json!({
+                    "a": {
+                        "file1": "",
+                        "file2": "",
+                        "file3": "",
+                        "file4": "",
+                    },
+                }),
+            )
+            .await;
+
+        let project = Project::test(app_state.fs.clone(), ["/root".as_ref()], cx).await;
+        let (_, workspace) = cx.add_window(|cx| Workspace::new(project.clone(), cx));
+        let pane = workspace.read_with(cx, |workspace, _| workspace.active_pane().clone());
+
+        let entries = cx.read(|cx| workspace.file_project_paths(cx));
+        let file1 = entries[0].clone();
+        let file2 = entries[1].clone();
+        let file3 = entries[2].clone();
+        let file4 = entries[3].clone();
+
+        let file1_item_id = workspace
+            .update(cx, |w, cx| w.open_path(file1.clone(), true, cx))
+            .await
+            .unwrap()
+            .id();
+        let file2_item_id = workspace
+            .update(cx, |w, cx| w.open_path(file2.clone(), true, cx))
+            .await
+            .unwrap()
+            .id();
+        let file3_item_id = workspace
+            .update(cx, |w, cx| w.open_path(file3.clone(), true, cx))
+            .await
+            .unwrap()
+            .id();
+        let file4_item_id = workspace
+            .update(cx, |w, cx| w.open_path(file4.clone(), true, cx))
+            .await
+            .unwrap()
+            .id();
+        assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
+
+        // Close all the pane items in some arbitrary order.
+        workspace
+            .update(cx, |workspace, cx| {
+                Pane::close_item(workspace, pane.clone(), file1_item_id, cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| {
+                Pane::close_item(workspace, pane.clone(), file4_item_id, cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| {
+                Pane::close_item(workspace, pane.clone(), file2_item_id, cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| {
+                Pane::close_item(workspace, pane.clone(), file3_item_id, cx)
+            })
+            .await
+            .unwrap();
+        assert_eq!(active_path(&workspace, cx), None);
+
+        // Reopen all the closed items, ensuring they are reopened in the same order
+        // in which they were closed.
+        workspace
+            .update(cx, |workspace, cx| Pane::reopen_closed_item(workspace, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::reopen_closed_item(workspace, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file2.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::reopen_closed_item(workspace, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::reopen_closed_item(workspace, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file1.clone()));
+
+        // Reopening past the last closed item is a no-op.
+        workspace
+            .update(cx, |workspace, cx| Pane::reopen_closed_item(workspace, cx))
+            .await;
+        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))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file2.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file4.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file3.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file2.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file1.clone()));
+
+        workspace
+            .update(cx, |workspace, cx| Pane::go_back(workspace, None, cx))
+            .await;
+        assert_eq!(active_path(&workspace, cx), Some(file1.clone()));
+
+        fn active_path(
+            workspace: &ViewHandle<Workspace>,
+            cx: &TestAppContext,
+        ) -> Option<ProjectPath> {
+            workspace.read_with(cx, |workspace, cx| {
+                let item = workspace.active_item(cx)?;
+                item.project_path(cx)
+            })
+        }
+    }
+
     #[gpui::test]
     fn test_bundled_themes(cx: &mut MutableAppContext) {
         let themes = ThemeRegistry::new(Assets, cx.font_cache().clone());