From bce49383500d04a63f4f3a845c2917ee9be89825 Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 8 Nov 2024 01:19:54 +0200 Subject: [PATCH] Improve outline panel keyboard navigation (#20385) Closes https://github.com/zed-industries/zed/issues/20187 Make outline panel more eager to open its entries: * scroll editor to selected outline entries (before it required an extra `"outline_panel::Open", { "change_selection": false }` action call) * make any `Open` action call to behave like `"outline_panel::Open", { "change_selection": true }` and remove the redundant parameter. Now opening an entry is equal to double clicking the same entry: the editor gets scrolled and its selection changes * add a way to open entries the same way as excerpts are open in multi buffers (will open the entire file, scroll and place the caret) * additionally, fix another race issue that caused wrong entry to be revealed after the selection change Release Notes: - Improved outline panel keyboard navigation --- assets/keymaps/default-linux.json | 6 +- assets/keymaps/default-macos.json | 6 +- crates/editor/src/editor.rs | 5 +- crates/outline_panel/src/outline_panel.rs | 269 +++++++++++++++++++--- 4 files changed, 254 insertions(+), 32 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 0ba76fba3f..764d07f840 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -564,9 +564,11 @@ "ctrl-alt-c": "outline_panel::CopyPath", "alt-ctrl-shift-c": "outline_panel::CopyRelativePath", "alt-ctrl-r": "outline_panel::RevealInFileManager", - "space": ["outline_panel::Open", { "change_selection": false }], + "space": "outline_panel::Open", "shift-down": "menu::SelectNext", - "shift-up": "menu::SelectPrev" + "shift-up": "menu::SelectPrev", + "alt-enter": "editor::OpenExcerpts", + "ctrl-k enter": "editor::OpenExcerptsSplit" } }, { diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 964af3ce3d..d0968e755b 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -577,9 +577,11 @@ "cmd-alt-c": "outline_panel::CopyPath", "alt-cmd-shift-c": "outline_panel::CopyRelativePath", "alt-cmd-r": "outline_panel::RevealInFileManager", - "space": ["outline_panel::Open", { "change_selection": false }], + "space": "outline_panel::Open", "shift-down": "menu::SelectNext", - "shift-up": "menu::SelectPrev" + "shift-up": "menu::SelectPrev", + "alt-enter": "editor::OpenExcerpts", + "cmd-k enter": "editor::OpenExcerptsSplit" } }, { diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 205d94b5a9..4663213134 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -49,6 +49,7 @@ pub mod test; use ::git::diff::DiffHunkStatus; pub(crate) use actions::*; +pub use actions::{OpenExcerpts, OpenExcerptsSplit}; use aho_corasick::AhoCorasick; use anyhow::{anyhow, Context as _, Result}; use blink_manager::BlinkManager; @@ -12539,11 +12540,11 @@ impl Editor { }); } - fn open_excerpts_in_split(&mut self, _: &OpenExcerptsSplit, cx: &mut ViewContext) { + pub fn open_excerpts_in_split(&mut self, _: &OpenExcerptsSplit, cx: &mut ViewContext) { self.open_excerpts_common(true, cx) } - fn open_excerpts(&mut self, _: &OpenExcerpts, cx: &mut ViewContext) { + pub fn open_excerpts(&mut self, _: &OpenExcerpts, cx: &mut ViewContext) { self.open_excerpts_common(false, cx) } diff --git a/crates/outline_panel/src/outline_panel.rs b/crates/outline_panel/src/outline_panel.rs index 4b75a03b1a..6e2c944802 100644 --- a/crates/outline_panel/src/outline_panel.rs +++ b/crates/outline_panel/src/outline_panel.rs @@ -23,9 +23,9 @@ use editor::{ use file_icons::FileIcons; use fuzzy::{match_strings, StringMatch, StringMatchCandidate}; use gpui::{ - actions, anchored, deferred, div, impl_actions, point, px, size, uniform_list, Action, - AnyElement, AppContext, AssetSource, AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, - Div, ElementId, EventEmitter, FocusHandle, FocusableView, HighlightStyle, InteractiveElement, + actions, anchored, deferred, div, point, px, size, uniform_list, Action, AnyElement, + AppContext, AssetSource, AsyncWindowContext, Bounds, ClipboardItem, DismissEvent, Div, + ElementId, EventEmitter, FocusHandle, FocusableView, HighlightStyle, InteractiveElement, IntoElement, KeyContext, ListHorizontalSizingBehavior, ListSizingBehavior, Model, MouseButton, MouseDownEvent, ParentElement, Pixels, Point, Render, SharedString, Stateful, StatefulInteractiveElement as _, Styled, Subscription, Task, UniformListScrollHandle, View, @@ -58,13 +58,6 @@ use workspace::{ }; use worktree::{Entry, ProjectEntryId, WorktreeId}; -#[derive(Clone, Default, Deserialize, PartialEq)] -pub struct Open { - change_selection: bool, -} - -impl_actions!(outline_panel, [Open]); - actions!( outline_panel, [ @@ -75,9 +68,10 @@ actions!( ExpandAllEntries, ExpandSelectedEntry, FoldDirectory, - ToggleActiveEditorPin, + Open, RevealInFileManager, SelectParent, + ToggleActiveEditorPin, ToggleFocus, UnfoldDirectory, ] @@ -813,11 +807,11 @@ impl OutlinePanel { self.update_cached_entries(None, cx); } - fn open(&mut self, open: &Open, cx: &mut ViewContext) { + fn open(&mut self, _: &Open, cx: &mut ViewContext) { if self.filter_editor.focus_handle(cx).is_focused(cx) { cx.propagate() } else if let Some(selected_entry) = self.selected_entry().cloned() { - self.open_entry(&selected_entry, open.change_selection, cx); + self.open_entry(&selected_entry, true, cx); } } @@ -834,6 +828,32 @@ impl OutlinePanel { } } + fn open_excerpts(&mut self, action: &editor::OpenExcerpts, cx: &mut ViewContext) { + if self.filter_editor.focus_handle(cx).is_focused(cx) { + cx.propagate() + } else if let Some((active_editor, selected_entry)) = + self.active_editor().zip(self.selected_entry().cloned()) + { + self.open_entry(&selected_entry, true, cx); + active_editor.update(cx, |editor, cx| editor.open_excerpts(action, cx)); + } + } + + fn open_excerpts_split( + &mut self, + action: &editor::OpenExcerptsSplit, + cx: &mut ViewContext, + ) { + if self.filter_editor.focus_handle(cx).is_focused(cx) { + cx.propagate() + } else if let Some((active_editor, selected_entry)) = + self.active_editor().zip(self.selected_entry().cloned()) + { + self.open_entry(&selected_entry, true, cx); + active_editor.update(cx, |editor, cx| editor.open_excerpts_in_split(action, cx)); + } + } + fn open_entry( &mut self, entry: &PanelEntry, @@ -851,7 +871,6 @@ impl OutlinePanel { Point::new(0.0, -(active_editor.read(cx).file_header_size() as f32)) }; - self.toggle_expanded(entry, cx); let scroll_target = match entry { PanelEntry::FoldedDirs(..) | PanelEntry::Fs(FsEntry::Directory(..)) => None, PanelEntry::Fs(FsEntry::ExternalFile(buffer_id, _)) => { @@ -949,6 +968,9 @@ impl OutlinePanel { } else { self.select_first(&SelectFirst {}, cx) } + if let Some(selected_entry) = self.selected_entry().cloned() { + self.open_entry(&selected_entry, false, cx); + } } fn select_prev(&mut self, _: &SelectPrev, cx: &mut ViewContext) { @@ -965,6 +987,9 @@ impl OutlinePanel { } else { self.select_last(&SelectLast, cx) } + if let Some(selected_entry) = self.selected_entry().cloned() { + self.open_entry(&selected_entry, false, cx); + } } fn select_parent(&mut self, _: &SelectParent, cx: &mut ViewContext) { @@ -1449,10 +1474,7 @@ impl OutlinePanel { } fn reveal_entry_for_selection(&mut self, editor: View, cx: &mut ViewContext<'_, Self>) { - if !self.active { - return; - } - if !OutlinePanelSettings::get_global(cx).auto_reveal_entries { + if !self.active || !OutlinePanelSettings::get_global(cx).auto_reveal_entries { return; } let project = self.project.clone(); @@ -2005,6 +2027,7 @@ impl OutlinePanel { return; } let change_selection = event.down.click_count > 1; + outline_panel.toggle_expanded(&clicked_entry, cx); outline_panel.open_entry(&clicked_entry, change_selection, cx); }) }) @@ -2447,19 +2470,20 @@ impl OutlinePanel { } fn clear_previous(&mut self, cx: &mut WindowContext<'_>) { + self.fs_entries_update_task = Task::ready(()); + self.outline_fetch_tasks.clear(); + self.cached_entries_update_task = Task::ready(()); + self.reveal_selection_task = Task::ready(Ok(())); self.filter_editor.update(cx, |editor, cx| editor.clear(cx)); self.collapsed_entries.clear(); self.unfolded_dirs.clear(); - self.selected_entry = SelectedEntry::None; - self.fs_entries_update_task = Task::ready(()); - self.cached_entries_update_task = Task::ready(()); self.active_item = None; self.fs_entries.clear(); self.fs_entries_depth.clear(); self.fs_children_count.clear(); - self.outline_fetch_tasks.clear(); self.excerpts.clear(); self.cached_entries = Vec::new(); + self.selected_entry = SelectedEntry::None; self.pinned = false; self.mode = ItemsDisplayMode::Outline; } @@ -2488,10 +2512,6 @@ impl OutlinePanel { .matches .iter() .rev() - .filter(|(match_range, _)| { - match_range.start.excerpt_id == excerpt_id - || match_range.end.excerpt_id == excerpt_id - }) .min_by_key(|&(match_range, _)| { let match_display_range = match_range.clone().to_display_points(&editor_snapshot); @@ -4244,6 +4264,8 @@ impl Render for OutlinePanel { .on_action(cx.listener(Self::toggle_active_editor_pin)) .on_action(cx.listener(Self::unfold_directory)) .on_action(cx.listener(Self::fold_directory)) + .on_action(cx.listener(Self::open_excerpts)) + .on_action(cx.listener(Self::open_excerpts_split)) .when(is_local, |el| { el.on_action(cx.listener(Self::reveal_in_finder)) }) @@ -4725,6 +4747,189 @@ mod tests { }); } + #[gpui::test(iterations = 10)] + async fn test_item_opening(cx: &mut TestAppContext) { + init_test(cx); + + let fs = FakeFs::new(cx.background_executor.clone()); + populate_with_test_ra_project(&fs, "/rust-analyzer").await; + let project = Project::test(fs.clone(), ["/rust-analyzer".as_ref()], cx).await; + project.read_with(cx, |project, _| { + project.languages().add(Arc::new(rust_lang())) + }); + let workspace = add_outline_panel(&project, cx).await; + let cx = &mut VisualTestContext::from_window(*workspace, cx); + let outline_panel = outline_panel(&workspace, cx); + outline_panel.update(cx, |outline_panel, cx| outline_panel.set_active(true, cx)); + + workspace + .update(cx, |workspace, cx| { + ProjectSearchView::deploy_search(workspace, &workspace::DeploySearch::default(), cx) + }) + .unwrap(); + let search_view = workspace + .update(cx, |workspace, cx| { + workspace + .active_pane() + .read(cx) + .items() + .find_map(|item| item.downcast::()) + .expect("Project search view expected to appear after new search event trigger") + }) + .unwrap(); + + let query = "param_names_for_lifetime_elision_hints"; + perform_project_search(&search_view, query, cx); + search_view.update(cx, |search_view, cx| { + search_view + .results_editor() + .update(cx, |results_editor, cx| { + assert_eq!( + results_editor.display_text(cx).match_indices(query).count(), + 9 + ); + }); + }); + let all_matches = r#"/ + crates/ + ide/src/ + inlay_hints/ + fn_lifetime_fn.rs + search: match config.param_names_for_lifetime_elision_hints { + search: allocated_lifetimes.push(if config.param_names_for_lifetime_elision_hints { + search: Some(it) if config.param_names_for_lifetime_elision_hints => { + search: InlayHintsConfig { param_names_for_lifetime_elision_hints: true, ..TEST_CONFIG }, + inlay_hints.rs + search: pub param_names_for_lifetime_elision_hints: bool, + search: param_names_for_lifetime_elision_hints: self + static_index.rs + search: param_names_for_lifetime_elision_hints: false, + rust-analyzer/src/ + cli/ + analysis_stats.rs + search: param_names_for_lifetime_elision_hints: true, + config.rs + search: param_names_for_lifetime_elision_hints: self"#; + let select_first_in_all_matches = |line_to_select: &str| { + assert!(all_matches.contains(line_to_select)); + all_matches.replacen( + line_to_select, + &format!("{line_to_select}{SELECTED_MARKER}"), + 1, + ) + }; + cx.executor() + .advance_clock(UPDATE_DEBOUNCE + Duration::from_millis(100)); + cx.run_until_parked(); + + let active_editor = outline_panel.update(cx, |outline_panel, _| { + outline_panel + .active_editor() + .expect("should have an active editor open") + }); + let initial_outline_selection = + "search: match config.param_names_for_lifetime_elision_hints {"; + outline_panel.update(cx, |outline_panel, cx| { + assert_eq!( + display_entries( + &snapshot(&outline_panel, cx), + &outline_panel.cached_entries, + outline_panel.selected_entry(), + ), + select_first_in_all_matches(initial_outline_selection) + ); + assert_eq!( + selected_row_text(&active_editor, cx), + initial_outline_selection.replace("search: ", ""), // Clear outline metadata prefixes + "Should place the initial editor selection on the corresponding search result" + ); + + outline_panel.select_next(&SelectNext, cx); + outline_panel.select_next(&SelectNext, cx); + }); + + let navigated_outline_selection = + "search: Some(it) if config.param_names_for_lifetime_elision_hints => {"; + outline_panel.update(cx, |outline_panel, cx| { + assert_eq!( + display_entries( + &snapshot(&outline_panel, cx), + &outline_panel.cached_entries, + outline_panel.selected_entry(), + ), + select_first_in_all_matches(navigated_outline_selection) + ); + assert_eq!( + selected_row_text(&active_editor, cx), + initial_outline_selection.replace("search: ", ""), // Clear outline metadata prefixes + "Should still have the initial caret position after SelectNext calls" + ); + }); + + outline_panel.update(cx, |outline_panel, cx| { + outline_panel.open(&Open, cx); + }); + outline_panel.update(cx, |_, cx| { + assert_eq!( + selected_row_text(&active_editor, cx), + navigated_outline_selection.replace("search: ", ""), // Clear outline metadata prefixes + "After opening, should move the caret to the opened outline entry's position" + ); + }); + + outline_panel.update(cx, |outline_panel, cx| { + outline_panel.select_next(&SelectNext, cx); + }); + let next_navigated_outline_selection = + "search: InlayHintsConfig { param_names_for_lifetime_elision_hints: true, ..TEST_CONFIG },"; + outline_panel.update(cx, |outline_panel, cx| { + assert_eq!( + display_entries( + &snapshot(&outline_panel, cx), + &outline_panel.cached_entries, + outline_panel.selected_entry(), + ), + select_first_in_all_matches(next_navigated_outline_selection) + ); + assert_eq!( + selected_row_text(&active_editor, cx), + navigated_outline_selection.replace("search: ", ""), // Clear outline metadata prefixes + "Should again preserve the selection after another SelectNext call" + ); + }); + + outline_panel.update(cx, |outline_panel, cx| { + outline_panel.open_excerpts(&editor::OpenExcerpts, cx); + }); + cx.executor() + .advance_clock(UPDATE_DEBOUNCE + Duration::from_millis(100)); + cx.run_until_parked(); + let new_active_editor = outline_panel.update(cx, |outline_panel, _| { + outline_panel + .active_editor() + .expect("should have an active editor open") + }); + outline_panel.update(cx, |outline_panel, cx| { + assert_ne!( + active_editor, new_active_editor, + "After opening an excerpt, new editor should be open" + ); + assert_eq!( + display_entries( + &snapshot(&outline_panel, cx), + &outline_panel.cached_entries, + outline_panel.selected_entry(), + ), + "fn_lifetime_fn.rs <==== selected" + ); + assert_eq!( + selected_row_text(&new_active_editor, cx), + next_navigated_outline_selection.replace("search: ", ""), // Clear outline metadata prefixes + "When opening the excerpt, should navigate to the place corresponding the outline entry" + ); + }); + } + #[gpui::test(iterations = 10)] async fn test_frontend_repo_structure(cx: &mut TestAppContext) { init_test(cx); @@ -5194,4 +5399,16 @@ mod tests { .read(cx) .snapshot(cx) } + + fn selected_row_text(editor: &View, cx: &mut WindowContext) -> String { + editor.update(cx, |editor, cx| { + let selections = editor.selections.all::(cx); + assert_eq!(selections.len(), 1, "Active editor should have exactly one selection after any outline panel interactions"); + let selection = selections.first().unwrap(); + let multi_buffer_snapshot = editor.buffer().read(cx).snapshot(cx); + let line_start = language::Point::new(selection.start.row, 0); + let line_end = multi_buffer_snapshot.clip_point(language::Point::new(selection.end.row, u32::MAX), language::Bias::Right); + multi_buffer_snapshot.text_for_range(line_start..line_end).collect::().trim().to_owned() + }) + } }