From 3c4f753e423d64174c178063ad320456ed2a0d0b Mon Sep 17 00:00:00 2001 From: dinocosta Date: Sat, 9 Aug 2025 18:11:15 +0100 Subject: [PATCH 1/5] feat(editor): context menu aside scrolling 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` --- assets/keymaps/default-linux.json | 6 +- assets/keymaps/default-macos.json | 6 +- assets/keymaps/vim.json | 9 ++ crates/editor/src/actions.rs | 17 ++++ crates/editor/src/code_context_menus.rs | 43 ++++++++- crates/editor/src/editor.rs | 18 ++++ crates/editor/src/editor_tests.rs | 113 ++++++++++++++++++++++++ crates/editor/src/element.rs | 1 + 8 files changed, 209 insertions(+), 4 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 708432393c..733f7c039e 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -702,7 +702,11 @@ "bindings": { "enter": "editor::ConfirmCompletion", "shift-enter": "editor::ConfirmCompletionReplace", - "tab": "editor::ComposeCompletion" + "tab": "editor::ComposeCompletion", + "ctrl-d": ["editor::ContextMenuScrollAside", { "pages": 0.5 }], + "ctrl-u": ["editor::ContextMenuScrollAside", { "pages": -0.5 }], + "ctrl-e": ["editor::ContextMenuScrollAside", { "lines": 1.0 }], + "ctrl-y": ["editor::ContextMenuScrollAside", { "lines": -1.0 }] } }, // Bindings for accepting edit predictions diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index abb741af29..128656b33f 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -805,7 +805,11 @@ "down": "editor::ContextMenuNext", "ctrl-n": "editor::ContextMenuNext", "pageup": "editor::ContextMenuFirst", - "pagedown": "editor::ContextMenuLast" + "pagedown": "editor::ContextMenuLast", + "ctrl-d": ["editor::ContextMenuScrollAside", { "pages": 0.5 }], + "ctrl-u": ["editor::ContextMenuScrollAside", { "pages": -0.5 }], + "ctrl-e": ["editor::ContextMenuScrollAside", { "lines": 1.0 }], + "ctrl-y": ["editor::ContextMenuScrollAside", { "lines": -1.0 }] } }, { diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 57edb1e4c1..a2271dbafe 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -476,6 +476,15 @@ "ctrl-n": "editor::SignatureHelpNext" } }, + { + "context": "vim_mode == insert && showing_completions", + "bindings": { + "ctrl-d": ["editor::ContextMenuScrollAside", { "pages": 0.5 }], + "ctrl-u": ["editor::ContextMenuScrollAside", { "pages": -0.5 }], + "ctrl-e": ["editor::ContextMenuScrollAside", { "lines": 1.0 }], + "ctrl-y": ["editor::ContextMenuScrollAside", { "lines": -1.0 }] + } + }, { "context": "vim_mode == replace", "bindings": { diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 39433b3c27..03a641f1aa 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -291,6 +291,19 @@ pub struct GoToPreviousDiagnostic { pub severity: GoToDiagnosticSeverityFilter, } +/// Scrolls the aside content of the context menu by the number of provided +/// lines, where positive values scroll the content down and negative values +/// scroll the content up. +#[derive(PartialEq, Clone, Default, Debug, Deserialize, JsonSchema, Action)] +#[action(namespace = editor)] +#[serde(deny_unknown_fields)] +pub struct ContextMenuScrollAside { + #[serde(default)] + pub lines: f32, + #[serde(default)] + pub pages: f32, +} + actions!( debugger, [ @@ -352,6 +365,10 @@ actions!( ContextMenuNext, /// Navigates to the previous item in the context menu. ContextMenuPrevious, + /// Scrolls the documentation displayed on the side of the context menu down + ContextMenuDocsScrollDown, + /// Scrolls the documentation displayed on the side of the context menu up + ContextMenuDocsScrollUp, /// Converts indentation from tabs to spaces. ConvertIndentationToSpaces, /// Converts indentation from spaces to tabs. diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index 4ae2a14ca7..f033d06905 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(crate) 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(), @@ -1177,6 +1199,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/editor.rs b/crates/editor/src/editor.rs index d1bf95c794..4b942fa6a8 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -73,6 +73,7 @@ pub use multi_buffer::{ pub use proposed_changes_editor::{ ProposedChangeLocation, ProposedChangesEditor, ProposedChangesEditorToolbar, }; +use scroll::ScrollAmount; pub use text::Bias; use ::git::{ @@ -12786,6 +12787,23 @@ impl Editor { } } + pub fn context_menu_scroll_aside( + &mut self, + event: &ContextMenuScrollAside, + window: &mut Window, + cx: &mut Context, + ) { + let scroll_amount = match (event.pages, event.lines) { + (pages, _) if pages != 0.0 => ScrollAmount::Page(pages), + (_, lines) if lines != 0.0 => ScrollAmount::Line(lines), + (_, _) => ScrollAmount::Page(0.0), + }; + + if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { + context_menu.scroll_aside(scroll_amount, window, cx); + } + } + pub fn context_menu_last( &mut self, _: &ContextMenuLast, diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index b31963c9c8..5709bb4cda 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -24071,6 +24071,119 @@ async fn test_newline_replacement_in_single_line(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_completion_menu_scroll_aside(cx: &mut TestAppContext) { + init_test(cx, |_| {}); + + // In order to be able to test the shortcuts that should allow the user to + // scroll the aside content of the completions menu, we first need to load + // the keymap file, otherwise simulating the keystrokes will not actually + // dispatching the necessary action. + cx.update(|cx| { + let default_key_bindings = settings::KeymapFile::load_asset_allow_partial_failure( + "keymaps/default-macos.json", + cx, + ) + .expect("Should be able to load deafult MacOS Keymap"); + + cx.bind_keys(default_key_bindings); + }); + + // Setup the completion to be provided by the LSP so we can set the + // completion item's documentation with a string long enough to overflow the + // assigned vertical space. + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions { + trigger_characters: Some(vec![".".to_string()]), + ..Default::default() + }), + ..Default::default() + }, + 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ˇ"); + cx.simulate_keystroke("."); + cx.executor().run_until_parked(); + + // Verify that the completion menu is visible before attempting to scroll + // it's aside content. + 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"); + }; + + assert_eq!(completion_menu_entries(&menu), &["Test Item"]); + 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); + }); +} + #[track_caller] fn extract_color_inlays(editor: &Editor, cx: &App) -> Vec { editor diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index a7fd0abf88..46295e8467 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -558,6 +558,7 @@ impl EditorElement { register_action(editor, window, Editor::context_menu_prev); register_action(editor, window, Editor::context_menu_next); register_action(editor, window, Editor::context_menu_last); + register_action(editor, window, Editor::context_menu_scroll_aside); register_action(editor, window, Editor::display_cursor_names); register_action(editor, window, Editor::unique_lines_case_insensitive); register_action(editor, window, Editor::unique_lines_case_sensitive); From 391593a701489a7792f0b0fbce6ed9f0dea0cc9f Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 12 Aug 2025 11:28:47 -0600 Subject: [PATCH 2/5] Cleaner version Co-authored-by: dinocosta --- assets/keymaps/default-linux.json | 6 +----- assets/keymaps/default-macos.json | 6 +----- assets/keymaps/vim.json | 9 --------- crates/editor/src/actions.rs | 17 ----------------- crates/editor/src/editor.rs | 18 ------------------ crates/editor/src/element.rs | 1 - crates/editor/src/hover_links.rs | 17 +++++++++++------ crates/editor/src/scroll/scroll_amount.rs | 2 +- crates/vim/src/normal/scroll.rs | 4 ++-- 9 files changed, 16 insertions(+), 64 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 733f7c039e..708432393c 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -702,11 +702,7 @@ "bindings": { "enter": "editor::ConfirmCompletion", "shift-enter": "editor::ConfirmCompletionReplace", - "tab": "editor::ComposeCompletion", - "ctrl-d": ["editor::ContextMenuScrollAside", { "pages": 0.5 }], - "ctrl-u": ["editor::ContextMenuScrollAside", { "pages": -0.5 }], - "ctrl-e": ["editor::ContextMenuScrollAside", { "lines": 1.0 }], - "ctrl-y": ["editor::ContextMenuScrollAside", { "lines": -1.0 }] + "tab": "editor::ComposeCompletion" } }, // Bindings for accepting edit predictions diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 128656b33f..abb741af29 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -805,11 +805,7 @@ "down": "editor::ContextMenuNext", "ctrl-n": "editor::ContextMenuNext", "pageup": "editor::ContextMenuFirst", - "pagedown": "editor::ContextMenuLast", - "ctrl-d": ["editor::ContextMenuScrollAside", { "pages": 0.5 }], - "ctrl-u": ["editor::ContextMenuScrollAside", { "pages": -0.5 }], - "ctrl-e": ["editor::ContextMenuScrollAside", { "lines": 1.0 }], - "ctrl-y": ["editor::ContextMenuScrollAside", { "lines": -1.0 }] + "pagedown": "editor::ContextMenuLast" } }, { diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index a2271dbafe..57edb1e4c1 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -476,15 +476,6 @@ "ctrl-n": "editor::SignatureHelpNext" } }, - { - "context": "vim_mode == insert && showing_completions", - "bindings": { - "ctrl-d": ["editor::ContextMenuScrollAside", { "pages": 0.5 }], - "ctrl-u": ["editor::ContextMenuScrollAside", { "pages": -0.5 }], - "ctrl-e": ["editor::ContextMenuScrollAside", { "lines": 1.0 }], - "ctrl-y": ["editor::ContextMenuScrollAside", { "lines": -1.0 }] - } - }, { "context": "vim_mode == replace", "bindings": { diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 03a641f1aa..39433b3c27 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -291,19 +291,6 @@ pub struct GoToPreviousDiagnostic { pub severity: GoToDiagnosticSeverityFilter, } -/// Scrolls the aside content of the context menu by the number of provided -/// lines, where positive values scroll the content down and negative values -/// scroll the content up. -#[derive(PartialEq, Clone, Default, Debug, Deserialize, JsonSchema, Action)] -#[action(namespace = editor)] -#[serde(deny_unknown_fields)] -pub struct ContextMenuScrollAside { - #[serde(default)] - pub lines: f32, - #[serde(default)] - pub pages: f32, -} - actions!( debugger, [ @@ -365,10 +352,6 @@ actions!( ContextMenuNext, /// Navigates to the previous item in the context menu. ContextMenuPrevious, - /// Scrolls the documentation displayed on the side of the context menu down - ContextMenuDocsScrollDown, - /// Scrolls the documentation displayed on the side of the context menu up - ContextMenuDocsScrollUp, /// Converts indentation from tabs to spaces. ConvertIndentationToSpaces, /// Converts indentation from spaces to tabs. diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 4b942fa6a8..d1bf95c794 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -73,7 +73,6 @@ pub use multi_buffer::{ pub use proposed_changes_editor::{ ProposedChangeLocation, ProposedChangesEditor, ProposedChangesEditorToolbar, }; -use scroll::ScrollAmount; pub use text::Bias; use ::git::{ @@ -12787,23 +12786,6 @@ impl Editor { } } - pub fn context_menu_scroll_aside( - &mut self, - event: &ContextMenuScrollAside, - window: &mut Window, - cx: &mut Context, - ) { - let scroll_amount = match (event.pages, event.lines) { - (pages, _) if pages != 0.0 => ScrollAmount::Page(pages), - (_, lines) if lines != 0.0 => ScrollAmount::Line(lines), - (_, _) => ScrollAmount::Page(0.0), - }; - - if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { - context_menu.scroll_aside(scroll_amount, window, cx); - } - } - pub fn context_menu_last( &mut self, _: &ContextMenuLast, diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 46295e8467..a7fd0abf88 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -558,7 +558,6 @@ impl EditorElement { register_action(editor, window, Editor::context_menu_prev); register_action(editor, window, Editor::context_menu_next); register_action(editor, window, Editor::context_menu_last); - register_action(editor, window, Editor::context_menu_scroll_aside); register_action(editor, window, Editor::display_cursor_names); register_action(editor, window, Editor::unique_lines_case_insensitive); register_action(editor, window, Editor::unique_lines_case_sensitive); diff --git a/crates/editor/src/hover_links.rs b/crates/editor/src/hover_links.rs index 02f93e6829..3a396b949a 100644 --- a/crates/editor/src/hover_links.rs +++ b/crates/editor/src/hover_links.rs @@ -188,22 +188,27 @@ 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); + return true; }; - popover.scroll(amount, window, cx); - true + if let Some(context_menu) = self.context_menu.borrow_mut().as_mut() { + context_menu.scroll_aside(amount, window, cx); + return true; + } + + return false; } fn cmd_click_reveal_task( diff --git a/crates/editor/src/scroll/scroll_amount.rs b/crates/editor/src/scroll/scroll_amount.rs index b2af4f8e4f..535f631f18 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 e2ae74b52b..a1a2ef1320 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(window, cx, |_, editor, window, 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, ) { From 9471ab994939d57ba797cff0e970e9f3231f433a Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 12 Aug 2025 11:32:49 -0600 Subject: [PATCH 3/5] Fix keymap --- assets/keymaps/vim.json | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/assets/keymaps/vim.json b/assets/keymaps/vim.json index 98f9cafc40..d58a5609ca 100644 --- a/assets/keymaps/vim.json +++ b/assets/keymaps/vim.json @@ -322,7 +322,7 @@ } }, { - "context": "vim_mode == insert", + "context": "vim_mode == insert && !menu", "bindings": { "ctrl-c": "vim::NormalBefore", "ctrl-[": "vim::NormalBefore", @@ -352,6 +352,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": { From cb2ca572e40821353961ef41c99c8e721255f1fb Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 12 Aug 2025 11:41:22 -0600 Subject: [PATCH 4/5] Fix test Co-authored-by: dinocosta --- crates/editor/src/code_context_menus.rs | 2 +- crates/editor/src/editor_tests.rs | 113 ------------------------ crates/vim/src/test.rs | 87 +++++++++++++++++- crates/vim/src/test/vim_test_context.rs | 4 + 4 files changed, 90 insertions(+), 116 deletions(-) diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index f033d06905..0f78fe6883 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -225,7 +225,7 @@ pub struct CompletionsMenu { scroll_handle: UniformListScrollHandle, // The `ScrollHandle` used on the Markdown documentation rendered on the // side of the completions menu. - pub(crate) scroll_handle_aside: ScrollHandle, + pub scroll_handle_aside: ScrollHandle, resolve_completions: bool, show_completion_documentation: bool, last_rendered_range: Rc>>>, diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 5709bb4cda..b31963c9c8 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -24071,119 +24071,6 @@ async fn test_newline_replacement_in_single_line(cx: &mut TestAppContext) { }); } -#[gpui::test] -async fn test_completion_menu_scroll_aside(cx: &mut TestAppContext) { - init_test(cx, |_| {}); - - // In order to be able to test the shortcuts that should allow the user to - // scroll the aside content of the completions menu, we first need to load - // the keymap file, otherwise simulating the keystrokes will not actually - // dispatching the necessary action. - cx.update(|cx| { - let default_key_bindings = settings::KeymapFile::load_asset_allow_partial_failure( - "keymaps/default-macos.json", - cx, - ) - .expect("Should be able to load deafult MacOS Keymap"); - - cx.bind_keys(default_key_bindings); - }); - - // Setup the completion to be provided by the LSP so we can set the - // completion item's documentation with a string long enough to overflow the - // assigned vertical space. - let mut cx = EditorLspTestContext::new_rust( - lsp::ServerCapabilities { - completion_provider: Some(lsp::CompletionOptions { - trigger_characters: Some(vec![".".to_string()]), - ..Default::default() - }), - ..Default::default() - }, - 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ˇ"); - cx.simulate_keystroke("."); - cx.executor().run_until_parked(); - - // Verify that the completion menu is visible before attempting to scroll - // it's aside content. - 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"); - }; - - assert_eq!(completion_menu_entries(&menu), &["Test Item"]); - 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); - }); -} - #[track_caller] fn extract_color_inlays(editor: &Editor, cx: &App) -> Vec { editor 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 904e48e5a3..b205c8dd16 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(), From 398f9665c9bc527a1298c1f50b6e8c22132a3116 Mon Sep 17 00:00:00 2001 From: Conrad Irwin Date: Tue, 12 Aug 2025 11:44:02 -0600 Subject: [PATCH 5/5] Clipster the original hipster --- crates/vim/src/normal/scroll.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vim/src/normal/scroll.rs b/crates/vim/src/normal/scroll.rs index 4d92a495bf..044d4e5545 100644 --- a/crates/vim/src/normal/scroll.rs +++ b/crates/vim/src/normal/scroll.rs @@ -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);