From 6192c83f23fae74366a370476afb1b2d88da048d Mon Sep 17 00:00:00 2001 From: Thorsten Ball Date: Wed, 18 Dec 2024 12:55:09 +0100 Subject: [PATCH] Show inline completions in menu only for supported providers (#22181) This changes #22093 by making the change only have an effect for the supported provider: Zeta. Made the change because the UX is still experimental and I don't want to break existing workflows for Copilot/Supermaven users. Even Zeta users can opt-out of it by setting `"show_inline_completions_in_menu": false` in their settings, in case they want the old show-inline-completion-or-show-lsp-completion behavior back. Release Notes: - N/A --- assets/settings/default.json | 3 + .../src/copilot_completion_provider.rs | 40 ++++--------- crates/editor/src/editor.rs | 59 ++++++++++++++----- crates/editor/src/editor_settings.rs | 7 +++ crates/editor/src/inline_completion_tests.rs | 4 ++ .../src/inline_completion.rs | 6 ++ .../src/supermaven_completion_provider.rs | 4 ++ crates/zeta/src/zeta.rs | 4 ++ 8 files changed, 85 insertions(+), 42 deletions(-) diff --git a/assets/settings/default.json b/assets/settings/default.json index 59156fe9cc..5041f79305 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -160,6 +160,9 @@ /// Whether to show the signature help after completion or a bracket pair inserted. /// If `auto_signature_help` is enabled, this setting will be treated as enabled also. "show_signature_help_after_edits": false, + /// Whether to show the inline completions next to the completions provided by a language server. + /// Only has an effect if inline completion provider supports it. + "show_inline_completions_in_menu": true, // Whether to show wrap guides (vertical rulers) in the editor. // Setting this to true will show a guide at the 'preferred_line_length' value // if 'soft_wrap' is set to 'preferred_line_length', and will show any diff --git a/crates/copilot/src/copilot_completion_provider.rs b/crates/copilot/src/copilot_completion_provider.rs index 7ef8d9ac7b..9283dcd198 100644 --- a/crates/copilot/src/copilot_completion_provider.rs +++ b/crates/copilot/src/copilot_completion_provider.rs @@ -59,6 +59,10 @@ impl InlineCompletionProvider for CopilotCompletionProvider { "Copilot" } + fn show_completions_in_menu() -> bool { + false + } + fn is_enabled( &self, buffer: &Model, @@ -326,17 +330,15 @@ mod tests { ); executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, cx| { - // We want to show both: the inline completion and the completion menu assert!(editor.context_menu_visible()); - assert!(editor.context_menu_contains_inline_completion()); - assert!(editor.has_active_inline_completion()); + assert!(!editor.context_menu_contains_inline_completion()); + assert!(!editor.has_active_inline_completion()); // Since we have both, the copilot suggestion is not shown inline assert_eq!(editor.text(cx), "one.\ntwo\nthree\n"); assert_eq!(editor.display_text(cx), "one.\ntwo\nthree\n"); // Confirming a non-copilot completion inserts it and hides the context menu, without showing // the copilot suggestion afterwards. - editor.context_menu_next(&Default::default(), cx); editor .confirm_completion(&Default::default(), cx) .unwrap() @@ -384,23 +386,12 @@ mod tests { // Ensure existing inline completion is interpolated when inserting again. cx.simulate_keystroke("c"); - drop(handle_completion_request( - &mut cx, - indoc! {" - one.c|<> - two - three - "}, - vec!["completion_a", "completion_b"], - )); executor.run_until_parked(); cx.update_editor(|editor, cx| { - // Since we have an LSP completion too, the inline completion is - // shown in the menu now - assert!(editor.context_menu_visible()); - assert!(editor.context_menu_contains_inline_completion()); + assert!(!editor.context_menu_visible()); + assert!(!editor.context_menu_contains_inline_completion()); assert!(editor.has_active_inline_completion()); - assert_eq!(editor.display_text(cx), "one.c\ntwo\nthree\n"); + assert_eq!(editor.display_text(cx), "one.copilot1\ntwo\nthree\n"); assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); }); @@ -416,16 +407,9 @@ mod tests { ); executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, cx| { - assert!(editor.context_menu_visible()); - assert!(editor.has_active_inline_completion()); - assert!(editor.context_menu_contains_inline_completion()); - assert_eq!(editor.display_text(cx), "one.c\ntwo\nthree\n"); - assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); - - // Canceling should first hide the menu and make Copilot suggestion visible. - editor.cancel(&Default::default(), cx); assert!(!editor.context_menu_visible()); assert!(editor.has_active_inline_completion()); + assert!(!editor.context_menu_contains_inline_completion()); assert_eq!(editor.display_text(cx), "one.copilot2\ntwo\nthree\n"); assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n"); @@ -928,8 +912,8 @@ mod tests { executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); cx.update_editor(|editor, cx| { assert!(editor.context_menu_visible()); - assert!(editor.context_menu_contains_inline_completion()); - assert!(editor.has_active_inline_completion(),); + assert!(!editor.context_menu_contains_inline_completion()); + assert!(!editor.has_active_inline_completion(),); assert_eq!(editor.text(cx), "one\ntwo.\nthree\n"); }); } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 8673e1f6d3..0a5649bdb2 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -2473,7 +2473,7 @@ impl Editor { } if self.hide_context_menu(cx).is_some() { - if self.has_active_inline_completion() { + if self.show_inline_completions_in_menu(cx) && self.has_active_inline_completion() { self.update_visible_inline_completion(cx); } return true; @@ -2856,6 +2856,7 @@ impl Editor { ); } + let had_active_inline_completion = this.has_active_inline_completion(); this.change_selections_inner(Some(Autoscroll::fit()), false, cx, |s| { s.select(new_selections) }); @@ -2876,7 +2877,9 @@ impl Editor { this.show_signature_help(&ShowSignatureHelp, cx); } - this.trigger_completion_on_input(&text, true, cx); + let trigger_in_words = + this.show_inline_completions_in_menu(cx) || !had_active_inline_completion; + this.trigger_completion_on_input(&text, trigger_in_words, cx); linked_editing_ranges::refresh_linked_ranges(this, cx); this.refresh_inline_completion(true, false, cx); }); @@ -3741,9 +3744,13 @@ impl Editor { let mut menu = menu.unwrap(); menu.resolve_selected_completion(editor.completion_provider.as_deref(), cx); - if let Some(hint) = editor.inline_completion_menu_hint(cx) { - editor.hide_active_inline_completion(cx); - menu.show_inline_completion_hint(hint); + if editor.show_inline_completions_in_menu(cx) { + if let Some(hint) = editor.inline_completion_menu_hint(cx) { + editor.hide_active_inline_completion(cx); + menu.show_inline_completion_hint(hint); + } + } else { + editor.discard_inline_completion(false, cx); } *editor.context_menu.borrow_mut() = @@ -3752,9 +3759,14 @@ impl Editor { cx.notify(); } else if editor.completion_tasks.len() <= 1 { // If there are no more completion tasks and the last menu was - // empty, we should hide it. If it was already hidden, we should - // also show the copilot completion when available. - editor.hide_context_menu(cx); + // empty, we should hide it. + let was_hidden = editor.hide_context_menu(cx).is_none(); + // If it was already hidden and we don't show inline + // completions in the menu, we should also show the + // inline-completion when available. + if was_hidden && editor.show_inline_completions_in_menu(cx) { + editor.update_visible_inline_completion(cx); + } } })?; @@ -3808,7 +3820,9 @@ impl Editor { return Some(Task::ready(Ok(()))); } CompletionEntry::Match(mat) => { - self.discard_inline_completion(true, cx); + if self.show_inline_completions_in_menu(cx) { + self.discard_inline_completion(true, cx); + } mat } }; @@ -4546,7 +4560,9 @@ impl Editor { _: &AcceptInlineCompletion, cx: &mut ViewContext, ) { - self.hide_context_menu(cx); + if self.show_inline_completions_in_menu(cx) { + self.hide_context_menu(cx); + } let Some(active_inline_completion) = self.active_inline_completion.as_ref() else { return; @@ -4713,7 +4729,11 @@ impl Editor { let offset_selection = selection.map(|endpoint| endpoint.to_offset(&multibuffer)); let excerpt_id = cursor.excerpt_id; - if !offset_selection.is_empty() + let completions_menu_has_precedence = !self.show_inline_completions_in_menu(cx) + && (self.context_menu.borrow().is_some() + || (!self.completion_tasks.is_empty() && !self.has_active_inline_completion())); + if completions_menu_has_precedence + || !offset_selection.is_empty() || self .active_inline_completion .as_ref() @@ -4771,7 +4791,7 @@ impl Editor { invalidation_row_range = edit_start_row..cursor_row; completion = InlineCompletion::Move(first_edit_start); } else { - if !self.has_active_completions_menu() { + if !self.show_inline_completions_in_menu(cx) || !self.has_active_completions_menu() { if edits .iter() .all(|(range, _)| range.to_offset(&multibuffer).is_empty()) @@ -4818,7 +4838,7 @@ impl Editor { invalidation_range, }); - if self.has_active_completions_menu() { + if self.show_inline_completions_in_menu(cx) && self.has_active_completions_menu() { if let Some(hint) = self.inline_completion_menu_hint(cx) { match self.context_menu.borrow_mut().as_mut() { Some(CodeContextMenu::Completions(menu)) => { @@ -4869,6 +4889,13 @@ impl Editor { Some(self.inline_completion_provider.as_ref()?.provider.clone()) } + fn show_inline_completions_in_menu(&self, cx: &AppContext) -> bool { + EditorSettings::get_global(cx).show_inline_completions_in_menu + && self + .inline_completion_provider() + .map_or(false, |provider| provider.show_completions_in_menu()) + } + fn render_code_actions_indicator( &self, _style: &EditorStyle, @@ -5137,7 +5164,11 @@ impl Editor { fn hide_context_menu(&mut self, cx: &mut ViewContext) -> Option { cx.notify(); self.completion_tasks.clear(); - self.context_menu.borrow_mut().take() + let context_menu = self.context_menu.borrow_mut().take(); + if context_menu.is_some() && !self.show_inline_completions_in_menu(cx) { + self.update_visible_inline_completion(cx); + } + context_menu } fn show_snippet_choices( diff --git a/crates/editor/src/editor_settings.rs b/crates/editor/src/editor_settings.rs index 0908c35262..d5cbb3e232 100644 --- a/crates/editor/src/editor_settings.rs +++ b/crates/editor/src/editor_settings.rs @@ -35,6 +35,7 @@ pub struct EditorSettings { pub auto_signature_help: bool, pub show_signature_help_after_edits: bool, pub jupyter: Jupyter, + pub show_inline_completions_in_menu: bool, } #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] @@ -300,6 +301,12 @@ pub struct EditorSettingsContent { /// Default: false pub show_signature_help_after_edits: Option, + /// Whether to show the inline completions next to the completions provided by a language server. + /// Only has an effect if inline completion provider supports it. + /// + /// Default: true + pub show_inline_completions_in_menu: Option, + /// Jupyter REPL settings. pub jupyter: Option, } diff --git a/crates/editor/src/inline_completion_tests.rs b/crates/editor/src/inline_completion_tests.rs index b1c922feba..58dbefb36f 100644 --- a/crates/editor/src/inline_completion_tests.rs +++ b/crates/editor/src/inline_completion_tests.rs @@ -321,6 +321,10 @@ impl InlineCompletionProvider for FakeInlineCompletionProvider { "Fake Completion Provider" } + fn show_completions_in_menu() -> bool { + false + } + fn is_enabled( &self, _buffer: &gpui::Model, diff --git a/crates/inline_completion/src/inline_completion.rs b/crates/inline_completion/src/inline_completion.rs index ff32fd5869..82323b50af 100644 --- a/crates/inline_completion/src/inline_completion.rs +++ b/crates/inline_completion/src/inline_completion.rs @@ -20,6 +20,7 @@ pub struct InlineCompletion { pub trait InlineCompletionProvider: 'static + Sized { fn name() -> &'static str; fn display_name() -> &'static str; + fn show_completions_in_menu() -> bool; fn is_enabled( &self, buffer: &Model, @@ -59,6 +60,7 @@ pub trait InlineCompletionProviderHandle { cursor_position: language::Anchor, cx: &AppContext, ) -> bool; + fn show_completions_in_menu(&self) -> bool; fn refresh( &self, buffer: Model, @@ -95,6 +97,10 @@ where T::display_name() } + fn show_completions_in_menu(&self) -> bool { + T::show_completions_in_menu() + } + fn is_enabled( &self, buffer: &Model, diff --git a/crates/supermaven/src/supermaven_completion_provider.rs b/crates/supermaven/src/supermaven_completion_provider.rs index 1f2fb5eb6c..4d8fa0f873 100644 --- a/crates/supermaven/src/supermaven_completion_provider.rs +++ b/crates/supermaven/src/supermaven_completion_provider.rs @@ -102,6 +102,10 @@ impl InlineCompletionProvider for SupermavenCompletionProvider { "Supermaven" } + fn show_completions_in_menu() -> bool { + false + } + fn is_enabled(&self, buffer: &Model, cursor_position: Anchor, cx: &AppContext) -> bool { if !self.supermaven.read(cx).is_enabled() { return false; diff --git a/crates/zeta/src/zeta.rs b/crates/zeta/src/zeta.rs index 2c0aab65b5..8789e87dab 100644 --- a/crates/zeta/src/zeta.rs +++ b/crates/zeta/src/zeta.rs @@ -932,6 +932,10 @@ impl inline_completion::InlineCompletionProvider for ZetaInlineCompletionProvide "Zeta" } + fn show_completions_in_menu() -> bool { + true + } + fn is_enabled( &self, buffer: &Model,