From 20625e98adb59a2c6e26a0591fa097755e722078 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner <53836821+bennetbo@users.noreply.github.com> Date: Mon, 29 Apr 2024 20:47:01 +0200 Subject: [PATCH] preview tabs: Allow replacing preview tab when using code navigation (#10730) This PR adds support for replacing the current preview tab when using GoToDefinition. Previously a tab, that was navigated away from, was converted into a permanent tab and the new tab was opened as preview. Without `enable_preview_from_code_navigation`: https://github.com/zed-industries/zed/assets/53836821/99840724-d6ff-4738-a9c4-ee71a0001634 With `enable_preview_from_code_navigation`: https://github.com/zed-industries/zed/assets/53836821/8c60efcb-d597-40bf-b08b-13faf5a289b6 Note: In the future I would like to improve support for the navigation history, because right now tabs that are not "normal" project items, e.g. FindAllReferences cannot be reopened Release Notes: - Added support for replacing the current preview tab when using code navigation (`enable_preview_from_code_navigation`) --- assets/settings/default.json | 6 +- .../src/activity_indicator.rs | 1 + crates/auto_update/src/auto_update.rs | 2 +- crates/collab/src/tests/following_tests.rs | 2 +- crates/command_palette/src/command_palette.rs | 2 +- crates/diagnostics/src/diagnostics.rs | 2 +- crates/editor/src/editor.rs | 20 ++++-- crates/editor/src/editor_tests.rs | 2 +- crates/editor/src/rust_analyzer_ext.rs | 1 + crates/extensions_ui/src/extensions_ui.rs | 2 +- crates/language_tools/src/lsp_log.rs | 1 + crates/search/src/project_search.rs | 5 +- crates/terminal_view/src/terminal_view.rs | 2 +- crates/welcome/src/welcome.rs | 2 +- crates/workspace/src/item.rs | 11 +++- crates/workspace/src/pane.rs | 38 ++++++----- crates/workspace/src/workspace.rs | 63 ++++++++++++------- crates/zed/src/zed.rs | 5 +- docs/src/configuring_zed.md | 13 +++- 19 files changed, 120 insertions(+), 60 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 3086843925..0c157e647d 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -331,8 +331,10 @@ // when you switch to another file unless you explicitly pin them. // This is useful for quickly viewing files without cluttering your workspace. "enabled": true, - // Whether to open files in preview mode when selected from the file finder. - "enable_preview_from_file_finder": false + // Whether to open tabs in preview mode when selected from the file finder. + "enable_preview_from_file_finder": false, + // Whether a preview tab gets replaced when code navigation is used to navigate away from the tab. + "enable_preview_from_code_navigation": false }, // Whether or not to remove any trailing whitespace from lines of a buffer // before saving it. diff --git a/crates/activity_indicator/src/activity_indicator.rs b/crates/activity_indicator/src/activity_indicator.rs index 58d6ee6191..4c5818afde 100644 --- a/crates/activity_indicator/src/activity_indicator.rs +++ b/crates/activity_indicator/src/activity_indicator.rs @@ -99,6 +99,7 @@ impl ActivityIndicator { Box::new( cx.new_view(|cx| Editor::for_buffer(buffer, Some(project.clone()), cx)), ), + None, cx, ); } diff --git a/crates/auto_update/src/auto_update.rs b/crates/auto_update/src/auto_update.rs index 5725efe39a..5fb5c99c7e 100644 --- a/crates/auto_update/src/auto_update.rs +++ b/crates/auto_update/src/auto_update.rs @@ -243,7 +243,7 @@ fn view_release_notes_locally(workspace: &mut Workspace, cx: &mut ViewContext( &ranges_to_highlight, @@ -8102,14 +8103,23 @@ impl Editor { cx, ); }); + let item = Box::new(editor); + let item_id = item.item_id(); + if split { workspace.split_item(SplitDirection::Right, item.clone(), cx); } else { - workspace.add_item_to_active_pane(item.clone(), cx); + let destination_index = workspace.active_pane().update(cx, |pane, cx| { + if PreviewTabsSettings::get_global(cx).enable_preview_from_code_navigation { + pane.close_current_preview_item(cx) + } else { + None + } + }); + workspace.add_item_to_active_pane(item.clone(), destination_index, cx); } - workspace.active_pane().clone().update(cx, |pane, cx| { - let item_id = item.item_id(); + workspace.active_pane().update(cx, |pane, cx| { pane.set_preview_item_id(Some(item_id), cx); }); } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 8e9e9d7299..f443146dd1 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9161,7 +9161,7 @@ async fn test_mutlibuffer_in_navigation_history(cx: &mut gpui::TestAppContext) { workspace.active_item(cx).is_none(), "active item should be None before the first item is added" ); - workspace.add_item_to_active_pane(Box::new(multi_buffer_editor.clone()), cx); + workspace.add_item_to_active_pane(Box::new(multi_buffer_editor.clone()), None, cx); let active_item = workspace .active_item(cx) .expect("should have an active item after adding the multi buffer"); diff --git a/crates/editor/src/rust_analyzer_ext.rs b/crates/editor/src/rust_analyzer_ext.rs index 1a950e0ea4..4544d1d785 100644 --- a/crates/editor/src/rust_analyzer_ext.rs +++ b/crates/editor/src/rust_analyzer_ext.rs @@ -106,6 +106,7 @@ pub fn expand_macro_recursively( }); workspace.add_item_to_active_pane( Box::new(cx.new_view(|cx| Editor::for_multibuffer(buffer, Some(project), cx))), + None, cx, ); }) diff --git a/crates/extensions_ui/src/extensions_ui.rs b/crates/extensions_ui/src/extensions_ui.rs index 34d9a2ca4d..7382d0b599 100644 --- a/crates/extensions_ui/src/extensions_ui.rs +++ b/crates/extensions_ui/src/extensions_ui.rs @@ -45,7 +45,7 @@ pub fn init(cx: &mut AppContext) { workspace.activate_item(&existing, cx); } else { let extensions_page = ExtensionsPage::new(workspace, cx); - workspace.add_item_to_active_pane(Box::new(extensions_page), cx) + workspace.add_item_to_active_pane(Box::new(extensions_page), None, cx) } }) .register_action(move |_, _: &InstallDevExtension, cx| { diff --git a/crates/language_tools/src/lsp_log.rs b/crates/language_tools/src/lsp_log.rs index 429e0c278c..a35d8b33e5 100644 --- a/crates/language_tools/src/lsp_log.rs +++ b/crates/language_tools/src/lsp_log.rs @@ -97,6 +97,7 @@ pub fn init(cx: &mut AppContext) { Box::new(cx.new_view(|cx| { LspLogView::new(workspace.project().clone(), log_store.clone(), cx) })), + None, cx, ); } diff --git a/crates/search/src/project_search.rs b/crates/search/src/project_search.rs index c75e5ac6a6..f7f659f78a 100644 --- a/crates/search/src/project_search.rs +++ b/crates/search/src/project_search.rs @@ -738,7 +738,7 @@ impl ProjectSearchView { let model = cx.new_model(|cx| ProjectSearch::new(workspace.project().clone(), cx)); let search = cx.new_view(|cx| ProjectSearchView::new(model, cx, None)); - workspace.add_item_to_active_pane(Box::new(search.clone()), cx); + workspace.add_item_to_active_pane(Box::new(search.clone()), None, cx); search.update(cx, |search, cx| { search .included_files_editor @@ -789,6 +789,7 @@ impl ProjectSearchView { }); workspace.add_item_to_active_pane( Box::new(cx.new_view(|cx| ProjectSearchView::new(model, cx, None))), + None, cx, ); } @@ -838,7 +839,7 @@ impl ProjectSearchView { let model = cx.new_model(|cx| ProjectSearch::new(workspace.project().clone(), cx)); let view = cx.new_view(|cx| ProjectSearchView::new(model, cx, settings)); - workspace.add_item_to_active_pane(Box::new(view.clone()), cx); + workspace.add_item_to_active_pane(Box::new(view.clone()), None, cx); view }; diff --git a/crates/terminal_view/src/terminal_view.rs b/crates/terminal_view/src/terminal_view.rs index 6c3908305f..1f81dad829 100644 --- a/crates/terminal_view/src/terminal_view.rs +++ b/crates/terminal_view/src/terminal_view.rs @@ -134,7 +134,7 @@ impl TerminalView { cx, ) }); - workspace.add_item_to_active_pane(Box::new(view), cx) + workspace.add_item_to_active_pane(Box::new(view), None, cx) } } diff --git a/crates/welcome/src/welcome.rs b/crates/welcome/src/welcome.rs index 62a82b2b54..5b9befdad2 100644 --- a/crates/welcome/src/welcome.rs +++ b/crates/welcome/src/welcome.rs @@ -29,7 +29,7 @@ pub fn init(cx: &mut AppContext) { cx.observe_new_views(|workspace: &mut Workspace, _cx| { workspace.register_action(|workspace, _: &Welcome, cx| { let welcome_page = WelcomePage::new(workspace, cx); - workspace.add_item_to_active_pane(Box::new(welcome_page), cx) + workspace.add_item_to_active_pane(Box::new(welcome_page), None, cx) }); }) .detach(); diff --git a/crates/workspace/src/item.rs b/crates/workspace/src/item.rs index e45c6ffe13..246cc8fe2d 100644 --- a/crates/workspace/src/item.rs +++ b/crates/workspace/src/item.rs @@ -45,6 +45,7 @@ pub struct ItemSettings { pub struct PreviewTabsSettings { pub enabled: bool, pub enable_preview_from_file_finder: bool, + pub enable_preview_from_code_navigation: bool, } #[derive(Clone, Default, Serialize, Deserialize, JsonSchema)] @@ -78,15 +79,19 @@ pub struct ItemSettingsContent { #[derive(Clone, Default, Serialize, Deserialize, JsonSchema)] pub struct PreviewTabsSettingsContent { - /// Whether to show opened editors as preview editors. - /// Preview editors do not stay open, are reused until explicitly set to be kept open opened (via double-click or editing) and show file names in italic. + /// Whether to show opened editors as preview tabs. + /// Preview tabs do not stay open, are reused until explicitly set to be kept open opened (via double-click or editing) and show file names in italic. /// /// Default: true enabled: Option, - /// Whether to open a preview editor when opening a file using the file finder. + /// Whether to open tabs in preview mode when selected from the file finder. /// /// Default: false enable_preview_from_file_finder: Option, + /// Whether a preview tab gets replaced when code navigation is used to navigate away from the tab. + /// + /// Default: false + enable_preview_from_code_navigation: Option, } impl Settings for ItemSettings { diff --git a/crates/workspace/src/pane.rs b/crates/workspace/src/pane.rs index 59c208924a..183ab933a2 100644 --- a/crates/workspace/src/pane.rs +++ b/crates/workspace/src/pane.rs @@ -623,21 +623,13 @@ impl Pane { self.activate_item(index, focus_item, focus_item, cx); existing_item } else { - let mut destination_index = None; - if allow_preview { - // If we are opening a new item as preview and we have an existing preview tab, remove it. - if let Some(item_idx) = self.preview_item_idx() { - let prev_active_item_index = self.active_item_index; - self.remove_item(item_idx, false, false, cx); - self.active_item_index = prev_active_item_index; - - // If the item is being opened as preview and we have an existing preview tab, - // open the new item in the position of the existing preview tab. - if item_idx < self.items.len() { - destination_index = Some(item_idx); - } - } - } + // If the item is being opened as preview and we have an existing preview tab, + // open the new item in the position of the existing preview tab. + let destination_index = if allow_preview { + self.close_current_preview_item(cx) + } else { + None + }; let new_item = build_item(cx); @@ -651,6 +643,22 @@ impl Pane { } } + pub fn close_current_preview_item(&mut self, cx: &mut ViewContext) -> Option { + let Some(item_idx) = self.preview_item_idx() else { + return None; + }; + + let prev_active_item_index = self.active_item_index; + self.remove_item(item_idx, false, false, cx); + self.active_item_index = prev_active_item_index; + + if item_idx < self.items.len() { + Some(item_idx) + } else { + None + } + } + pub fn add_item( &mut self, item: Box, diff --git a/crates/workspace/src/workspace.rs b/crates/workspace/src/workspace.rs index dbeb7eb427..9b42e5da9f 100644 --- a/crates/workspace/src/workspace.rs +++ b/crates/workspace/src/workspace.rs @@ -2061,14 +2061,20 @@ impl Workspace { } } - pub fn add_item_to_active_pane(&mut self, item: Box, cx: &mut WindowContext) { - self.add_item(self.active_pane.clone(), item, cx) + pub fn add_item_to_active_pane( + &mut self, + item: Box, + destination_index: Option, + cx: &mut WindowContext, + ) { + self.add_item(self.active_pane.clone(), item, destination_index, cx) } pub fn add_item( &mut self, pane: View, item: Box, + destination_index: Option, cx: &mut WindowContext, ) { if let Some(text) = item.telemetry_event_text(cx) { @@ -2077,7 +2083,9 @@ impl Workspace { .report_app_event(format!("{}: open", text)); } - pane.update(cx, |pane, cx| pane.add_item(item, true, true, None, cx)); + pane.update(cx, |pane, cx| { + pane.add_item(item, true, true, destination_index, cx) + }); } pub fn split_item( @@ -2087,7 +2095,7 @@ impl Workspace { cx: &mut ViewContext, ) { let new_pane = self.split_pane(self.active_pane.clone(), split_direction, cx); - self.add_item(new_pane, item, cx); + self.add_item(new_pane, item, None, cx); } pub fn open_abs_path( @@ -2259,10 +2267,21 @@ impl Workspace { } let item = cx.new_view(|cx| T::for_project_item(self.project().clone(), project_item, cx)); + + let item_id = item.item_id(); + let mut destination_index = None; pane.update(cx, |pane, cx| { + if PreviewTabsSettings::get_global(cx).enable_preview_from_code_navigation { + if let Some(preview_item_id) = pane.preview_item_id() { + if preview_item_id != item_id { + destination_index = pane.close_current_preview_item(cx); + } + } + } pane.set_preview_item_id(Some(item.item_id()), cx) }); - self.add_item(pane, Box::new(item.clone()), cx); + + self.add_item(pane, Box::new(item.clone()), destination_index, cx); item } @@ -5157,7 +5176,7 @@ mod tests { item }); workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item1.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item1.clone()), None, cx); }); item1.update(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(0))); @@ -5169,7 +5188,7 @@ mod tests { item }); workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item2.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item2.clone()), None, cx); }); item1.update(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(1))); item2.update(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(1))); @@ -5183,7 +5202,7 @@ mod tests { item }); workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item3.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item3.clone()), None, cx); }); item1.update(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(1))); item2.update(cx, |item, _| assert_eq!(item.tab_detail.get(), Some(3))); @@ -5227,7 +5246,7 @@ mod tests { // Add an item to an empty pane workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item1), cx) + workspace.add_item_to_active_pane(Box::new(item1), None, cx) }); project.update(cx, |project, cx| { assert_eq!( @@ -5241,7 +5260,7 @@ mod tests { // Add a second item to a non-empty pane workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item2), cx) + workspace.add_item_to_active_pane(Box::new(item2), None, cx) }); assert_eq!(cx.window_title().as_deref(), Some("two.txt — root1")); project.update(cx, |project, cx| { @@ -5296,7 +5315,7 @@ mod tests { // When there are no dirty items, there's nothing to do. let item1 = cx.new_view(|cx| TestItem::new(cx)); workspace.update(cx, |w, cx| { - w.add_item_to_active_pane(Box::new(item1.clone()), cx) + w.add_item_to_active_pane(Box::new(item1.clone()), None, cx) }); let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx)); assert!(task.await.unwrap()); @@ -5310,8 +5329,8 @@ mod tests { .with_project_items(&[TestProjectItem::new(1, "1.txt", cx)]) }); workspace.update(cx, |w, cx| { - w.add_item_to_active_pane(Box::new(item2.clone()), cx); - w.add_item_to_active_pane(Box::new(item3.clone()), cx); + w.add_item_to_active_pane(Box::new(item2.clone()), None, cx); + w.add_item_to_active_pane(Box::new(item3.clone()), None, cx); }); let task = workspace.update(cx, |w, cx| w.prepare_to_close(false, cx)); cx.executor().run_until_parked(); @@ -5355,10 +5374,10 @@ mod tests { .with_project_items(&[TestProjectItem::new_untitled(cx)]) }); let pane = workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item1.clone()), cx); - workspace.add_item_to_active_pane(Box::new(item2.clone()), cx); - workspace.add_item_to_active_pane(Box::new(item3.clone()), cx); - workspace.add_item_to_active_pane(Box::new(item4.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item1.clone()), None, cx); + workspace.add_item_to_active_pane(Box::new(item2.clone()), None, cx); + workspace.add_item_to_active_pane(Box::new(item3.clone()), None, cx); + workspace.add_item_to_active_pane(Box::new(item4.clone()), None, cx); workspace.active_pane().clone() }); @@ -5480,9 +5499,9 @@ mod tests { // multi-entry items: (3, 4) let left_pane = workspace.update(cx, |workspace, cx| { let left_pane = workspace.active_pane().clone(); - workspace.add_item_to_active_pane(Box::new(item_2_3.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item_2_3.clone()), None, cx); for item in single_entry_items { - workspace.add_item_to_active_pane(Box::new(item), cx); + workspace.add_item_to_active_pane(Box::new(item), None, cx); } left_pane.update(cx, |pane, cx| { pane.activate_item(2, true, true, cx); @@ -5553,7 +5572,7 @@ mod tests { }); let item_id = item.entity_id(); workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item.clone()), None, cx); }); // Autosave on window change. @@ -5638,7 +5657,7 @@ mod tests { // Add the item again, ensuring autosave is prevented if the underlying file has been deleted. workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item.clone()), None, cx); }); item.update(cx, |item, cx| { item.project_items[0].update(cx, |item, _| { @@ -5676,7 +5695,7 @@ mod tests { let toolbar_notify_count = Rc::new(RefCell::new(0)); workspace.update(cx, |workspace, cx| { - workspace.add_item_to_active_pane(Box::new(item.clone()), cx); + workspace.add_item_to_active_pane(Box::new(item.clone()), None, cx); let toolbar_notification_count = toolbar_notify_count.clone(); cx.observe(&toolbar, move |_, _, _| { *toolbar_notification_count.borrow_mut() += 1 diff --git a/crates/zed/src/zed.rs b/crates/zed/src/zed.rs index 7ec9b68731..dbd6a34774 100644 --- a/crates/zed/src/zed.rs +++ b/crates/zed/src/zed.rs @@ -618,7 +618,7 @@ fn open_log_file(workspace: &mut Workspace, cx: &mut ViewContext) { }) }); - workspace.add_item_to_active_pane(Box::new(editor), cx); + workspace.add_item_to_active_pane(Box::new(editor), None, cx); }) .log_err(); }) @@ -837,7 +837,7 @@ fn open_telemetry_log_file(workspace: &mut Workspace, cx: &mut ViewContext