From e9252a7a74b6af4002639405b32b6167da810fe6 Mon Sep 17 00:00:00 2001 From: Dino Date: Fri, 29 Aug 2025 21:23:44 +0100 Subject: [PATCH] editor: Context menu aside scrolling (#35985) Add support for scrolling the contents rendered aside an `editor::code_context_menus::CodeContextMenu` by introducing the `scroll_aside` method. For now this method is only implemented for the `CodeContextMenu::Completions` variant, which will scroll the aside contents for an `editor::code_context_menus::CompletionsMenu` element, as a `ScrollHandle` is added to the aside content that is rendered. In order to be possible to trigger this via keybindings, a new editor action is introduced, `ContextMenuScrollAside`, which accepts a number of lines or pages to scroll the content by. Lastly, the default keymaps for both MacOS and Linux, as well as for Zed's vim mode, are updated to ensure that the following keybindings are supported when a completion menu is open and the completion item's documentation is rendered aside: - `ctrl-e` - `ctrl-y` - `ctrl-d` - `ctrl-u` ### Recording https://github.com/user-attachments/assets/02043763-87ea-46f5-9768-00e907127b69 --- Closes #13194 Release Notes: - Added support for scrolling the documentation panel shown alongside the completion menu in the editor with `cltr-d`, `ctrl-u`, `ctrl-e` and `ctrl-y` --------- Co-authored-by: Conrad Irwin Co-authored-by: MrSubidubi --- assets/keymaps/vim.json | 11 ++- crates/editor/src/code_context_menus.rs | 43 ++++++++++- crates/editor/src/hover_links.rs | 18 +++-- crates/editor/src/hover_popover.rs | 2 +- crates/editor/src/scroll/scroll_amount.rs | 2 +- crates/vim/src/normal/scroll.rs | 6 +- crates/vim/src/test.rs | 87 ++++++++++++++++++++++- crates/vim/src/test/vim_test_context.rs | 4 ++ 8 files changed, 156 insertions(+), 17 deletions(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 0a88baee02..bd6eb3982c 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -324,7 +324,7 @@ } }, { - "context": "vim_mode == insert", + "context": "vim_mode == insert && !menu", "bindings": { "ctrl-c": "vim::NormalBefore", "ctrl-[": "vim::NormalBefore", @@ -354,6 +354,15 @@ "ctrl-s": "editor::ShowSignatureHelp" } }, + { + "context": "showing_completions", + "bindings": { + "ctrl-d": "vim::ScrollDown", + "ctrl-u": "vim::ScrollUp", + "ctrl-e": "vim::LineDown", + "ctrl-y": "vim::LineUp" + } + }, { "context": "(vim_mode == normal || vim_mode == helix_normal) && !menu", "bindings": { diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index 96809d6877..01e74284ef 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -1,7 +1,9 @@ +use crate::scroll::ScrollAmount; use fuzzy::{StringMatch, StringMatchCandidate}; use gpui::{ - AnyElement, Entity, Focusable, FontWeight, ListSizingBehavior, ScrollStrategy, SharedString, - Size, StrikethroughStyle, StyledText, Task, UniformListScrollHandle, div, px, uniform_list, + AnyElement, Entity, Focusable, FontWeight, ListSizingBehavior, ScrollHandle, ScrollStrategy, + SharedString, Size, StrikethroughStyle, StyledText, Task, UniformListScrollHandle, div, px, + uniform_list, }; use itertools::Itertools; use language::CodeLabel; @@ -184,6 +186,20 @@ impl CodeContextMenu { CodeContextMenu::CodeActions(_) => false, } } + + pub fn scroll_aside( + &mut self, + scroll_amount: ScrollAmount, + window: &mut Window, + cx: &mut Context, + ) { + match self { + CodeContextMenu::Completions(completions_menu) => { + completions_menu.scroll_aside(scroll_amount, window, cx) + } + CodeContextMenu::CodeActions(_) => (), + } + } } pub enum ContextMenuOrigin { @@ -207,6 +223,9 @@ pub struct CompletionsMenu { filter_task: Task<()>, cancel_filter: Arc, scroll_handle: UniformListScrollHandle, + // The `ScrollHandle` used on the Markdown documentation rendered on the + // side of the completions menu. + pub scroll_handle_aside: ScrollHandle, resolve_completions: bool, show_completion_documentation: bool, last_rendered_range: Rc>>>, @@ -279,6 +298,7 @@ impl CompletionsMenu { filter_task: Task::ready(()), cancel_filter: Arc::new(AtomicBool::new(false)), scroll_handle: UniformListScrollHandle::new(), + scroll_handle_aside: ScrollHandle::new(), resolve_completions: true, last_rendered_range: RefCell::new(None).into(), markdown_cache: RefCell::new(VecDeque::new()).into(), @@ -348,6 +368,7 @@ impl CompletionsMenu { filter_task: Task::ready(()), cancel_filter: Arc::new(AtomicBool::new(false)), scroll_handle: UniformListScrollHandle::new(), + scroll_handle_aside: ScrollHandle::new(), resolve_completions: false, show_completion_documentation: false, last_rendered_range: RefCell::new(None).into(), @@ -911,6 +932,7 @@ impl CompletionsMenu { .max_w(max_size.width) .max_h(max_size.height) .overflow_y_scroll() + .track_scroll(&self.scroll_handle_aside) .occlude(), ) .into_any_element(), @@ -1175,6 +1197,23 @@ impl CompletionsMenu { } }); } + + pub fn scroll_aside( + &mut self, + amount: ScrollAmount, + window: &mut Window, + cx: &mut Context, + ) { + let mut offset = self.scroll_handle_aside.offset(); + + offset.y -= amount.pixels( + window.line_height(), + self.scroll_handle_aside.bounds().size.height - px(16.), + ) / 2.0; + + cx.notify(); + self.scroll_handle_aside.set_offset(offset); + } } #[derive(Clone)] diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index 94f49f601a..ba0b6f8868 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -188,22 +188,26 @@ impl Editor { pub fn scroll_hover( &mut self, - amount: &ScrollAmount, + amount: ScrollAmount, window: &mut Window, cx: &mut Context, ) -> bool { let selection = self.selections.newest_anchor().head(); let snapshot = self.snapshot(window, cx); - let Some(popover) = self.hover_state.info_popovers.iter().find(|popover| { + if let Some(popover) = self.hover_state.info_popovers.iter().find(|popover| { popover .symbol_range .point_within_range(&TriggerPoint::Text(selection), &snapshot) - }) else { - return false; - }; - popover.scroll(amount, window, cx); - true + }) { + popover.scroll(amount, window, cx); + true + } else if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { + context_menu.scroll_aside(amount, window, cx); + true + } else { + false + } } fn cmd_click_reveal_task( diff --git a/crates/editor/src/hover_popover.rs b/crates/editor/src/hover_popover.rs index fab5345787..6541f76a56 100644 --- a/crates/editor/src/hover_popover.rs +++ b/crates/editor/src/hover_popover.rs @@ -896,7 +896,7 @@ impl InfoPopover { .into_any_element() } - pub fn scroll(&self, amount: &ScrollAmount, window: &mut Window, cx: &mut Context) { + pub fn scroll(&self, amount: ScrollAmount, window: &mut Window, cx: &mut Context) { let mut current = self.scroll_handle.offset(); current.y -= amount.pixels( window.line_height(), diff --git a/crates/editor/src/scroll/scroll_amount.rs b/crates/editor/src/scroll/scroll_amount.rs index 5992c9023c..43f1aa1285 100644 --- a/crates/editor/src/scroll/scroll_amount.rs +++ b/crates/editor/src/scroll/scroll_amount.rs @@ -15,7 +15,7 @@ impl ScrollDirection { } } -#[derive(Debug, Clone, PartialEq, Deserialize)] +#[derive(Debug, Clone, Copy, PartialEq, Deserialize)] pub enum ScrollAmount { // Scroll N lines (positive is towards the end of the document) Line(f32), diff --git a/crates/vim/src/normal/scroll.rs b/crates/vim/src/normal/scroll.rs index 9eb8367f57..eeb98692bc 100644 --- a/crates/vim/src/normal/scroll.rs +++ b/crates/vim/src/normal/scroll.rs @@ -98,7 +98,7 @@ impl Vim { Vim::take_forced_motion(cx); self.exit_temporary_normal(window, cx); self.update_editor(cx, |_, editor, cx| { - scroll_editor(editor, move_cursor, &amount, window, cx) + scroll_editor(editor, move_cursor, amount, window, cx) }); } } @@ -106,7 +106,7 @@ impl Vim { fn scroll_editor( editor: &mut Editor, preserve_cursor_position: bool, - amount: &ScrollAmount, + amount: ScrollAmount, window: &mut Window, cx: &mut Context, ) { @@ -126,7 +126,7 @@ fn scroll_editor( ScrollAmount::Line(amount.lines(visible_line_count) - 1.0) } } - _ => amount.clone(), + _ => amount, }; editor.scroll_screen(&amount, window, cx); diff --git a/crates/vim/src/test.rs b/crates/vim/src/test.rs index ce04b621cb..84376719d1 100644 --- a/crates/vim/src/test.rs +++ b/crates/vim/src/test.rs @@ -8,13 +8,15 @@ use collections::HashMap; use command_palette::CommandPalette; use editor::{ AnchorRangeExt, DisplayPoint, Editor, EditorMode, MultiBuffer, actions::DeleteLine, - display_map::DisplayRow, test::editor_test_context::EditorTestContext, + code_context_menus::CodeContextMenu, display_map::DisplayRow, + test::editor_test_context::EditorTestContext, }; use futures::StreamExt; -use gpui::{KeyBinding, Modifiers, MouseButton, TestAppContext}; +use gpui::{KeyBinding, Modifiers, MouseButton, TestAppContext, px}; use language::Point; pub use neovim_backed_test_context::*; use settings::SettingsStore; +use ui::Pixels; use util::test::marked_text_ranges; pub use vim_test_context::*; @@ -971,6 +973,87 @@ async fn test_comma_w(cx: &mut gpui::TestAppContext) { .assert_eq("hellˇo hello\nhello hello"); } +#[gpui::test] +async fn test_completion_menu_scroll_aside(cx: &mut TestAppContext) { + let mut cx = VimTestContext::new_typescript(cx).await; + + cx.lsp + .set_request_handler::(move |_, _| async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + lsp::CompletionItem { + label: "Test Item".to_string(), + documentation: Some(lsp::Documentation::String( + "This is some very long documentation content that will be displayed in the aside panel for scrolling.\n".repeat(50) + )), + ..Default::default() + }, + ]))) + }); + + cx.set_state("variableˇ", Mode::Insert); + cx.simulate_keystroke("."); + cx.executor().run_until_parked(); + + let mut initial_offset: Pixels = px(0.0); + + cx.update_editor(|editor, _, _| { + let binding = editor.context_menu().borrow(); + let Some(CodeContextMenu::Completions(menu)) = binding.as_ref() else { + panic!("Should have completions menu open"); + }; + + initial_offset = menu.scroll_handle_aside.offset().y; + }); + + // The `ctrl-e` shortcut should scroll the completion menu's aside content + // down, so the updated offset should be lower than the initial offset. + cx.simulate_keystroke("ctrl-e"); + cx.update_editor(|editor, _, _| { + let binding = editor.context_menu().borrow(); + let Some(CodeContextMenu::Completions(menu)) = binding.as_ref() else { + panic!("Should have completions menu open"); + }; + + assert!(menu.scroll_handle_aside.offset().y < initial_offset); + }); + + // The `ctrl-y` shortcut should do the inverse scrolling as `ctrl-e`, so the + // offset should now be the same as the initial offset. + cx.simulate_keystroke("ctrl-y"); + cx.update_editor(|editor, _, _| { + let binding = editor.context_menu().borrow(); + let Some(CodeContextMenu::Completions(menu)) = binding.as_ref() else { + panic!("Should have completions menu open"); + }; + + assert_eq!(menu.scroll_handle_aside.offset().y, initial_offset); + }); + + // The `ctrl-d` shortcut should scroll the completion menu's aside content + // down, so the updated offset should be lower than the initial offset. + cx.simulate_keystroke("ctrl-d"); + cx.update_editor(|editor, _, _| { + let binding = editor.context_menu().borrow(); + let Some(CodeContextMenu::Completions(menu)) = binding.as_ref() else { + panic!("Should have completions menu open"); + }; + + assert!(menu.scroll_handle_aside.offset().y < initial_offset); + }); + + // The `ctrl-u` shortcut should do the inverse scrolling as `ctrl-u`, so the + // offset should now be the same as the initial offset. + cx.simulate_keystroke("ctrl-u"); + cx.update_editor(|editor, _, _| { + let binding = editor.context_menu().borrow(); + let Some(CodeContextMenu::Completions(menu)) = binding.as_ref() else { + panic!("Should have completions menu open"); + }; + + assert_eq!(menu.scroll_handle_aside.offset().y, initial_offset); + }); +} + #[gpui::test] async fn test_rename(cx: &mut gpui::TestAppContext) { let mut cx = VimTestContext::new_typescript(cx).await; diff --git a/crates/vim/src/test/vim_test_context.rs b/crates/vim/src/test/vim_test_context.rs index e7ac692df1..ef9588acae 100644 --- a/crates/vim/src/test/vim_test_context.rs +++ b/crates/vim/src/test/vim_test_context.rs @@ -49,6 +49,10 @@ impl VimTestContext { Self::new_with_lsp( EditorLspTestContext::new_typescript( lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + ..Default::default() + }), rename_provider: Some(lsp::OneOf::Right(lsp::RenameOptions { prepare_provider: Some(true), work_done_progress_options: Default::default(),