From f700268029163c683fe8f62320e1ffc4a346d616 Mon Sep 17 00:00:00 2001 From: Michael Sloan Date: Fri, 7 Feb 2025 03:53:38 -0700 Subject: [PATCH] Improve vim interactions with edit predictions (#24418) * When an edit prediction is present in non-insertion modes, hide it but show `tab Jump to edit`. * Removes discarding of edit predictions when going from insert mode to normal mode, instead just hide them in non-insertion modes. * Removes zeta-specific showing of predictions in normal mode. This behavior was only happening in special cases anyway - where the discard of completions wasn't happening due to some other thing taking precedence in `dismiss_menus_and_popups`. Release Notes: - N/A --------- Co-authored-by: Conrad Co-authored-by: Mikayla --- .../src/copilot_completion_provider.rs | 4 - crates/editor/src/editor.rs | 60 +++++++++----- crates/editor/src/element.rs | 80 +++++++++++-------- crates/editor/src/inline_completion_tests.rs | 4 - .../src/inline_completion.rs | 6 -- .../src/supermaven_completion_provider.rs | 4 - crates/vim/src/vim.rs | 28 ++++--- crates/zeta/src/zeta.rs | 4 - 8 files changed, 102 insertions(+), 88 deletions(-) diff --git a/crates/copilot/src/copilot_completion_provider.rs b/crates/copilot/src/copilot_completion_provider.rs index 93ffeaf2e2..51aa112849 100644 --- a/crates/copilot/src/copilot_completion_provider.rs +++ b/crates/copilot/src/copilot_completion_provider.rs @@ -61,10 +61,6 @@ impl InlineCompletionProvider for CopilotCompletionProvider { false } - fn show_completions_in_normal_mode() -> bool { - false - } - fn is_refreshing(&self) -> bool { self.pending_refresh.is_some() } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index b2c87d63b5..29b266d970 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -679,9 +679,7 @@ pub struct Editor { active_inline_completion: Option, /// Used to prevent flickering as the user types while the menu is open stale_inline_completion_in_menu: Option, - // enable_inline_completions is a switch that Vim can use to disable - // edit predictions based on its mode. - show_inline_completions: bool, + inline_completions_hidden_for_vim_mode: bool, show_inline_completions_override: Option, menu_inline_completions_policy: MenuInlineCompletionsPolicy, previewing_inline_completion: bool, @@ -1390,8 +1388,8 @@ impl Editor { hovered_cursors: Default::default(), next_editor_action_id: EditorActionId::default(), editor_actions: Rc::default(), + inline_completions_hidden_for_vim_mode: false, show_inline_completions_override: None, - show_inline_completions: true, menu_inline_completions_policy: MenuInlineCompletionsPolicy::ByProvider, custom_context_menu: None, show_git_blame_gutter: false, @@ -1829,11 +1827,19 @@ impl Editor { self.input_enabled = input_enabled; } - pub fn set_show_inline_completions_enabled(&mut self, enabled: bool, cx: &mut Context) { - self.show_inline_completions = enabled; - if !self.show_inline_completions { - self.take_active_inline_completion(cx); - cx.notify(); + pub fn set_inline_completions_hidden_for_vim_mode( + &mut self, + hidden: bool, + window: &mut Window, + cx: &mut Context, + ) { + if hidden != self.inline_completions_hidden_for_vim_mode { + self.inline_completions_hidden_for_vim_mode = hidden; + if hidden { + self.update_visible_inline_completion(window, cx); + } else { + self.refresh_inline_completion(true, false, window, cx); + } } } @@ -1902,6 +1908,15 @@ impl Editor { self.refresh_inline_completion(false, true, window, cx); } + pub fn inline_completion_start_anchor(&self) -> Option { + let active_completion = self.active_inline_completion.as_ref()?; + let result = match &active_completion.completion { + InlineCompletion::Edit { edits, .. } => edits.first()?.0.start, + InlineCompletion::Move { target, .. } => *target, + }; + Some(result) + } + fn inline_completions_disabled_in_scope( &self, buffer: &Entity, @@ -2566,7 +2581,7 @@ impl Editor { pub fn dismiss_menus_and_popups( &mut self, - should_report_inline_completion_event: bool, + is_user_requested: bool, window: &mut Window, cx: &mut Context, ) -> bool { @@ -2590,7 +2605,7 @@ impl Editor { return true; } - if self.discard_inline_completion(should_report_inline_completion_event, cx) { + if is_user_requested && self.discard_inline_completion(true, cx) { return true; } @@ -4634,12 +4649,7 @@ impl Editor { } if !user_requested - && (!self.show_inline_completions - || !self.should_show_inline_completions_in_buffer( - &buffer, - cursor_buffer_position, - cx, - ) + && (!self.should_show_inline_completions_in_buffer(&buffer, cursor_buffer_position, cx) || !self.is_focused(window) || buffer.read(cx).is_empty()) { @@ -4752,7 +4762,7 @@ impl Editor { let cursor = self.selections.newest_anchor().head(); let (buffer, cursor_buffer_position) = self.buffer.read(cx).text_anchor_for_position(cursor, cx)?; - if !self.show_inline_completions + if self.inline_completions_hidden_for_vim_mode || !self.should_show_inline_completions_in_buffer(&buffer, cursor_buffer_position, cx) { return None; @@ -4873,6 +4883,7 @@ impl Editor { match &active_inline_completion.completion { InlineCompletion::Move { target, .. } => { let target = *target; + // Note that this is also done in vim's handler of the Tab action. self.change_selections(Some(Autoscroll::newest()), window, cx, |selections| { selections.select_anchor_ranges([target..target]); }); @@ -5083,7 +5094,6 @@ impl Editor { || (!self.completion_tasks.is_empty() && !self.has_active_inline_completion())); if completions_menu_has_precedence || !offset_selection.is_empty() - || !self.show_inline_completions || self .active_inline_completion .as_ref() @@ -5138,8 +5148,11 @@ impl Editor { } else { None }; - let completion = if let Some(move_invalidation_row_range) = move_invalidation_row_range { - invalidation_row_range = move_invalidation_row_range; + let is_move = + move_invalidation_row_range.is_some() || self.inline_completions_hidden_for_vim_mode; + let completion = if is_move { + invalidation_row_range = + move_invalidation_row_range.unwrap_or(edit_start_row..edit_end_row); let target = first_edit_start; let target_point = text::ToPoint::to_point(&target.text_anchor, &snapshot); // TODO: Base this off of TreeSitter or word boundaries? @@ -5158,7 +5171,10 @@ impl Editor { snapshot, } } else { - if !self.inline_completion_visible_in_cursor_popover(true, cx) { + let show_completions_in_buffer = !self + .inline_completion_visible_in_cursor_popover(true, cx) + && !self.inline_completions_hidden_for_vim_mode; + if show_completions_in_buffer { if edits .iter() .all(|(range, _)| range.to_offset(&multibuffer).is_empty()) diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index 1a56c8954a..ab211f3d3e 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -3636,7 +3636,7 @@ impl EditorElement { self.editor.focus_handle(cx), window, cx, - ); + )?; let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); let offset = point((text_bounds.size.width - size.width) / 2., PADDING_Y); element.prepaint_at(text_bounds.origin + offset, window, cx); @@ -3649,7 +3649,7 @@ impl EditorElement { self.editor.focus_handle(cx), window, cx, - ); + )?; let size = element.layout_as_root(AvailableSpace::min_size(), window, cx); let offset = point( (text_bounds.size.width - size.width) / 2., @@ -3665,7 +3665,7 @@ impl EditorElement { self.editor.focus_handle(cx), window, cx, - ); + )?; let target_line_end = DisplayPoint::new( target_display_point.row(), @@ -3740,7 +3740,7 @@ impl EditorElement { self.editor.focus_handle(cx), window, cx, - ); + )?; element.prepaint_as_root( text_bounds.origin + origin + point(PADDING_X, px(0.)), @@ -5742,24 +5742,32 @@ fn inline_completion_accept_indicator( focus_handle: FocusHandle, window: &Window, cx: &App, -) -> AnyElement { - let use_hardcoded_linux_preview_binding; +) -> Option { + let use_hardcoded_linux_bindings; #[cfg(target_os = "macos")] { - use_hardcoded_linux_preview_binding = false; + use_hardcoded_linux_bindings = false; } #[cfg(not(target_os = "macos"))] { - use_hardcoded_linux_preview_binding = previewing; + use_hardcoded_linux_bindings = true; } - let accept_keystroke = if use_hardcoded_linux_preview_binding { - Keystroke { - modifiers: Default::default(), - key: "enter".to_string(), - key_char: None, + let accept_keystroke = if use_hardcoded_linux_bindings { + if previewing { + Keystroke { + modifiers: Default::default(), + key: "enter".to_string(), + key_char: None, + } + } else { + Keystroke { + modifiers: Default::default(), + key: "tab".to_string(), + key_char: None, + } } } else { let bindings = window.bindings_for_action_in(&crate::AcceptInlineCompletion, &focus_handle); @@ -5767,10 +5775,10 @@ fn inline_completion_accept_indicator( .last() .and_then(|binding| binding.keystrokes().first()) { - // TODO: clone unnecessary once `use_hardcoded_linux_preview_binding` is removed. + // TODO: clone unnecessary once `use_hardcoded_linux_bindings` is removed. keystroke.clone() } else { - return div().into_any(); + return None; } }; @@ -5793,26 +5801,28 @@ fn inline_completion_accept_indicator( let padding_right = if icon.is_some() { px(4.) } else { px(8.) }; - h_flex() - .py_0p5() - .pl_1() - .pr(padding_right) - .gap_1() - .bg(cx.theme().colors().text_accent.opacity(0.15)) - .border_1() - .border_color(cx.theme().colors().text_accent.opacity(0.8)) - .rounded_md() - .shadow_sm() - .child(accept_key) - .child(Label::new(label).size(LabelSize::Small)) - .when_some(icon, |element, icon| { - element.child( - div() - .mt(px(1.5)) - .child(Icon::new(icon).size(IconSize::Small)), - ) - }) - .into_any() + Some( + h_flex() + .py_0p5() + .pl_1() + .pr(padding_right) + .gap_1() + .bg(cx.theme().colors().text_accent.opacity(0.15)) + .border_1() + .border_color(cx.theme().colors().text_accent.opacity(0.8)) + .rounded_md() + .shadow_sm() + .child(accept_key) + .child(Label::new(label).size(LabelSize::Small)) + .when_some(icon, |element, icon| { + element.child( + div() + .mt(px(1.5)) + .child(Icon::new(icon).size(IconSize::Small)), + ) + }) + .into_any(), + ) } #[allow(clippy::too_many_arguments)] diff --git a/crates/editor/src/inline_completion_tests.rs b/crates/editor/src/inline_completion_tests.rs index c0ad941b7a..a5b9e8a3e0 100644 --- a/crates/editor/src/inline_completion_tests.rs +++ b/crates/editor/src/inline_completion_tests.rs @@ -376,10 +376,6 @@ impl InlineCompletionProvider for FakeInlineCompletionProvider { false } - fn show_completions_in_normal_mode() -> bool { - false - } - fn is_enabled( &self, _buffer: &gpui::Entity, diff --git a/crates/inline_completion/src/inline_completion.rs b/crates/inline_completion/src/inline_completion.rs index d262112e03..7810d99785 100644 --- a/crates/inline_completion/src/inline_completion.rs +++ b/crates/inline_completion/src/inline_completion.rs @@ -42,7 +42,6 @@ pub trait InlineCompletionProvider: 'static + Sized { fn name() -> &'static str; fn display_name() -> &'static str; fn show_completions_in_menu() -> bool; - fn show_completions_in_normal_mode() -> bool; fn show_tab_accept_marker() -> bool { false } @@ -95,7 +94,6 @@ pub trait InlineCompletionProviderHandle { cx: &App, ) -> bool; fn show_completions_in_menu(&self) -> bool; - fn show_completions_in_normal_mode(&self) -> bool; fn show_tab_accept_marker(&self) -> bool; fn data_collection_state(&self, cx: &App) -> DataCollectionState; fn toggle_data_collection(&self, cx: &mut App); @@ -142,10 +140,6 @@ where T::show_completions_in_menu() } - fn show_completions_in_normal_mode(&self) -> bool { - T::show_completions_in_normal_mode() - } - fn show_tab_accept_marker(&self) -> bool { T::show_tab_accept_marker() } diff --git a/crates/supermaven/src/supermaven_completion_provider.rs b/crates/supermaven/src/supermaven_completion_provider.rs index c17053ca55..b14d7d54c2 100644 --- a/crates/supermaven/src/supermaven_completion_provider.rs +++ b/crates/supermaven/src/supermaven_completion_provider.rs @@ -110,10 +110,6 @@ impl InlineCompletionProvider for SupermavenCompletionProvider { false } - fn show_completions_in_normal_mode() -> bool { - false - } - fn is_enabled(&self, _buffer: &Entity, _cursor_position: Anchor, cx: &App) -> bool { self.supermaven.read(cx).is_enabled() } diff --git a/crates/vim/src/vim.rs b/crates/vim/src/vim.rs index e331260faa..f479acee31 100644 --- a/crates/vim/src/vim.rs +++ b/crates/vim/src/vim.rs @@ -23,6 +23,7 @@ use anyhow::Result; use collections::HashMap; use editor::{ movement::{self, FindRange}, + scroll::Autoscroll, Anchor, Bias, Editor, EditorEvent, EditorMode, ToPoint, }; use gpui::{ @@ -344,7 +345,19 @@ impl Vim { vim.push_count_digit(n.0, window, cx); }); Vim::action(editor, cx, |vim, _: &Tab, window, cx| { - vim.input_ignored(" ".into(), window, cx) + let Some(anchor) = vim + .editor() + .and_then(|editor| editor.read(cx).inline_completion_start_anchor()) + else { + return; + }; + + vim.update_editor(window, cx, |_, editor, window, cx| { + editor.change_selections(Some(Autoscroll::fit()), window, cx, |s| { + s.select_anchor_ranges([anchor..anchor]) + }); + }); + vim.switch_mode(Mode::Insert, true, window, cx); }); Vim::action(editor, cx, |vim, _: &Enter, window, cx| { vim.input_ignored("\n".into(), window, cx) @@ -1274,7 +1287,7 @@ impl Vim { } fn sync_vim_settings(&mut self, window: &mut Window, cx: &mut Context) { - self.update_editor(window, cx, |vim, editor, _, cx| { + self.update_editor(window, cx, |vim, editor, window, cx| { editor.set_cursor_shape(vim.cursor_shape(), cx); editor.set_clip_at_line_ends(vim.clip_at_line_ends(), cx); editor.set_collapse_matches(true); @@ -1282,14 +1295,11 @@ impl Vim { editor.set_autoindent(vim.should_autoindent()); editor.selections.line_mode = matches!(vim.mode, Mode::VisualLine); - let enable_inline_completions = match vim.mode { - Mode::Insert | Mode::Replace => true, - Mode::Normal => editor - .inline_completion_provider() - .map_or(false, |provider| provider.show_completions_in_normal_mode()), - _ => false, + let hide_inline_completions = match vim.mode { + Mode::Insert | Mode::Replace => false, + _ => true, }; - editor.set_show_inline_completions_enabled(enable_inline_completions, cx); + editor.set_inline_completions_hidden_for_vim_mode(hide_inline_completions, window, cx); }); cx.notify() } diff --git a/crates/zeta/src/zeta.rs b/crates/zeta/src/zeta.rs index a2d6601342..a2be4811fa 100644 --- a/crates/zeta/src/zeta.rs +++ b/crates/zeta/src/zeta.rs @@ -1513,10 +1513,6 @@ impl inline_completion::InlineCompletionProvider for ZetaInlineCompletionProvide true } - fn show_completions_in_normal_mode() -> bool { - true - } - fn show_tab_accept_marker() -> bool { true }