From 0652542f605bc3cecb9785f1bcd721280bb0349d Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 27 Jun 2022 17:44:33 +0200 Subject: [PATCH 1/3] Introduce `pane::ReopenClosedItem` bound to `cmd-shift-t` --- assets/keymaps/default.json | 1 + crates/workspace/src/pane.rs | 62 ++++++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 7 deletions(-) diff --git a/assets/keymaps/default.json b/assets/keymaps/default.json index 6b8bf4ffb2..0f1e005891 100644 --- a/assets/keymaps/default.json +++ b/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" } }, diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 09051d7266..4f2fe43c83 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -25,6 +25,7 @@ actions!( ActivateNextItem, CloseActiveItem, CloseInactiveItems, + ReopenClosedItem, SplitLeft, SplitUp, SplitRight, @@ -82,6 +83,7 @@ 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(Pane::reopen_closed_item); cx.add_action(|workspace: &mut Workspace, action: &GoBack, cx| { Pane::go_back( workspace, @@ -133,6 +135,7 @@ pub struct NavHistory { mode: NavigationMode, backward_stack: VecDeque, forward_stack: VecDeque, + closed_stack: VecDeque, paths_by_item: HashMap, } @@ -141,6 +144,8 @@ enum NavigationMode { Normal, GoingBack, GoingForward, + ClosingItem, + ReopeningClosed, Disabled, } @@ -200,6 +205,20 @@ impl Pane { ) } + pub fn reopen_closed_item( + workspace: &mut Workspace, + _: &ReopenClosedItem, + cx: &mut ViewContext, + ) { + Self::navigate_history( + workspace, + workspace.active_pane().clone(), + NavigationMode::ReopeningClosed, + cx, + ) + .detach(); + } + fn navigate_history( workspace: &mut Workspace, pane: ViewHandle, @@ -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 { + self.closed_stack.pop_back() + } + fn pop(&mut self, mode: NavigationMode) -> Option { 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::ReopeningClosed => self.pop_closed(), } } @@ -940,7 +979,7 @@ impl NavHistory { pub fn push(&mut self, data: Option, item: Rc) { match self.mode { NavigationMode::Disabled => {} - NavigationMode::Normal => { + NavigationMode::Normal | NavigationMode::ReopeningClosed => { 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), }); } + 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), + }); + } } } } From c6e7ae528ff0c2698fb11bb10c474b3251d1c172 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Mon, 27 Jun 2022 17:59:25 +0200 Subject: [PATCH 2/3] Add test for reopening closed items --- crates/workspace/src/pane.rs | 8 +- crates/zed/src/zed.rs | 164 +++++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+), 4 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 4f2fe43c83..0dc1bfdd10 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -83,7 +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(Pane::reopen_closed_item); + 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, @@ -207,16 +209,14 @@ impl Pane { pub fn reopen_closed_item( workspace: &mut Workspace, - _: &ReopenClosedItem, cx: &mut ViewContext, - ) { + ) -> Task<()> { Self::navigate_history( workspace, workspace.active_pane().clone(), NavigationMode::ReopeningClosed, cx, ) - .detach(); } fn navigate_history( diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index f3a9ae280f..68a713e9af 100644 --- a/crates/zed/src/zed.rs +++ b/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, + cx: &TestAppContext, + ) -> Option { + 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()); From 57f34c6992630425f841e6411731575a3ab106ec Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 28 Jun 2022 08:04:39 +0200 Subject: [PATCH 3/3] :lipstick: --- crates/workspace/src/pane.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 0dc1bfdd10..29770dc772 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -147,7 +147,7 @@ enum NavigationMode { GoingBack, GoingForward, ClosingItem, - ReopeningClosed, + ReopeningClosedItem, Disabled, } @@ -214,7 +214,7 @@ impl Pane { Self::navigate_history( workspace, workspace.active_pane().clone(), - NavigationMode::ReopeningClosed, + NavigationMode::ReopeningClosedItem, cx, ) } @@ -968,7 +968,7 @@ impl NavHistory { NavigationMode::Normal | NavigationMode::Disabled | NavigationMode::ClosingItem => None, NavigationMode::GoingBack => self.pop_backward(), NavigationMode::GoingForward => self.pop_forward(), - NavigationMode::ReopeningClosed => self.pop_closed(), + NavigationMode::ReopeningClosedItem => self.pop_closed(), } } @@ -979,7 +979,7 @@ impl NavHistory { pub fn push(&mut self, data: Option, item: Rc) { match self.mode { NavigationMode::Disabled => {} - NavigationMode::Normal | NavigationMode::ReopeningClosed => { + NavigationMode::Normal | NavigationMode::ReopeningClosedItem => { if self.backward_stack.len() >= MAX_NAVIGATION_HISTORY_LEN { self.backward_stack.pop_front(); }