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
This commit is contained in:
Thorsten Ball 2024-12-18 12:55:09 +01:00 committed by GitHub
parent e8c9283700
commit 6192c83f23
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 85 additions and 42 deletions

View file

@ -160,6 +160,9 @@
/// Whether to show the signature help after completion or a bracket pair inserted. /// 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. /// If `auto_signature_help` is enabled, this setting will be treated as enabled also.
"show_signature_help_after_edits": false, "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. // Whether to show wrap guides (vertical rulers) in the editor.
// Setting this to true will show a guide at the 'preferred_line_length' value // 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 // if 'soft_wrap' is set to 'preferred_line_length', and will show any

View file

@ -59,6 +59,10 @@ impl InlineCompletionProvider for CopilotCompletionProvider {
"Copilot" "Copilot"
} }
fn show_completions_in_menu() -> bool {
false
}
fn is_enabled( fn is_enabled(
&self, &self,
buffer: &Model<Buffer>, buffer: &Model<Buffer>,
@ -326,17 +330,15 @@ mod tests {
); );
executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT);
cx.update_editor(|editor, cx| { 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_visible());
assert!(editor.context_menu_contains_inline_completion()); assert!(!editor.context_menu_contains_inline_completion());
assert!(editor.has_active_inline_completion()); assert!(!editor.has_active_inline_completion());
// Since we have both, the copilot suggestion is not shown inline // Since we have both, the copilot suggestion is not shown inline
assert_eq!(editor.text(cx), "one.\ntwo\nthree\n"); assert_eq!(editor.text(cx), "one.\ntwo\nthree\n");
assert_eq!(editor.display_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 // Confirming a non-copilot completion inserts it and hides the context menu, without showing
// the copilot suggestion afterwards. // the copilot suggestion afterwards.
editor.context_menu_next(&Default::default(), cx);
editor editor
.confirm_completion(&Default::default(), cx) .confirm_completion(&Default::default(), cx)
.unwrap() .unwrap()
@ -384,23 +386,12 @@ mod tests {
// Ensure existing inline completion is interpolated when inserting again. // Ensure existing inline completion is interpolated when inserting again.
cx.simulate_keystroke("c"); cx.simulate_keystroke("c");
drop(handle_completion_request(
&mut cx,
indoc! {"
one.c|<>
two
three
"},
vec!["completion_a", "completion_b"],
));
executor.run_until_parked(); executor.run_until_parked();
cx.update_editor(|editor, cx| { cx.update_editor(|editor, cx| {
// Since we have an LSP completion too, the inline completion is assert!(!editor.context_menu_visible());
// shown in the menu now 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!(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"); assert_eq!(editor.text(cx), "one.c\ntwo\nthree\n");
}); });
@ -416,16 +407,9 @@ mod tests {
); );
executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT); executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT);
cx.update_editor(|editor, cx| { 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.context_menu_visible());
assert!(editor.has_active_inline_completion()); 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.display_text(cx), "one.copilot2\ntwo\nthree\n");
assert_eq!(editor.text(cx), "one.c\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); executor.advance_clock(COPILOT_DEBOUNCE_TIMEOUT);
cx.update_editor(|editor, cx| { cx.update_editor(|editor, cx| {
assert!(editor.context_menu_visible()); assert!(editor.context_menu_visible());
assert!(editor.context_menu_contains_inline_completion()); assert!(!editor.context_menu_contains_inline_completion());
assert!(editor.has_active_inline_completion(),); assert!(!editor.has_active_inline_completion(),);
assert_eq!(editor.text(cx), "one\ntwo.\nthree\n"); assert_eq!(editor.text(cx), "one\ntwo.\nthree\n");
}); });
} }

View file

@ -2473,7 +2473,7 @@ impl Editor {
} }
if self.hide_context_menu(cx).is_some() { 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); self.update_visible_inline_completion(cx);
} }
return true; 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| { this.change_selections_inner(Some(Autoscroll::fit()), false, cx, |s| {
s.select(new_selections) s.select(new_selections)
}); });
@ -2876,7 +2877,9 @@ impl Editor {
this.show_signature_help(&ShowSignatureHelp, cx); 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); linked_editing_ranges::refresh_linked_ranges(this, cx);
this.refresh_inline_completion(true, false, cx); this.refresh_inline_completion(true, false, cx);
}); });
@ -3741,9 +3744,13 @@ impl Editor {
let mut menu = menu.unwrap(); let mut menu = menu.unwrap();
menu.resolve_selected_completion(editor.completion_provider.as_deref(), cx); menu.resolve_selected_completion(editor.completion_provider.as_deref(), cx);
if let Some(hint) = editor.inline_completion_menu_hint(cx) { if editor.show_inline_completions_in_menu(cx) {
editor.hide_active_inline_completion(cx); if let Some(hint) = editor.inline_completion_menu_hint(cx) {
menu.show_inline_completion_hint(hint); editor.hide_active_inline_completion(cx);
menu.show_inline_completion_hint(hint);
}
} else {
editor.discard_inline_completion(false, cx);
} }
*editor.context_menu.borrow_mut() = *editor.context_menu.borrow_mut() =
@ -3752,9 +3759,14 @@ impl Editor {
cx.notify(); cx.notify();
} else if editor.completion_tasks.len() <= 1 { } else if editor.completion_tasks.len() <= 1 {
// If there are no more completion tasks and the last menu was // If there are no more completion tasks and the last menu was
// empty, we should hide it. If it was already hidden, we should // empty, we should hide it.
// also show the copilot completion when available. let was_hidden = editor.hide_context_menu(cx).is_none();
editor.hide_context_menu(cx); // 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(()))); return Some(Task::ready(Ok(())));
} }
CompletionEntry::Match(mat) => { CompletionEntry::Match(mat) => {
self.discard_inline_completion(true, cx); if self.show_inline_completions_in_menu(cx) {
self.discard_inline_completion(true, cx);
}
mat mat
} }
}; };
@ -4546,7 +4560,9 @@ impl Editor {
_: &AcceptInlineCompletion, _: &AcceptInlineCompletion,
cx: &mut ViewContext<Self>, cx: &mut ViewContext<Self>,
) { ) {
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 { let Some(active_inline_completion) = self.active_inline_completion.as_ref() else {
return; return;
@ -4713,7 +4729,11 @@ impl Editor {
let offset_selection = selection.map(|endpoint| endpoint.to_offset(&multibuffer)); let offset_selection = selection.map(|endpoint| endpoint.to_offset(&multibuffer));
let excerpt_id = cursor.excerpt_id; 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 || self
.active_inline_completion .active_inline_completion
.as_ref() .as_ref()
@ -4771,7 +4791,7 @@ impl Editor {
invalidation_row_range = edit_start_row..cursor_row; invalidation_row_range = edit_start_row..cursor_row;
completion = InlineCompletion::Move(first_edit_start); completion = InlineCompletion::Move(first_edit_start);
} else { } else {
if !self.has_active_completions_menu() { if !self.show_inline_completions_in_menu(cx) || !self.has_active_completions_menu() {
if edits if edits
.iter() .iter()
.all(|(range, _)| range.to_offset(&multibuffer).is_empty()) .all(|(range, _)| range.to_offset(&multibuffer).is_empty())
@ -4818,7 +4838,7 @@ impl Editor {
invalidation_range, 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) { if let Some(hint) = self.inline_completion_menu_hint(cx) {
match self.context_menu.borrow_mut().as_mut() { match self.context_menu.borrow_mut().as_mut() {
Some(CodeContextMenu::Completions(menu)) => { Some(CodeContextMenu::Completions(menu)) => {
@ -4869,6 +4889,13 @@ impl Editor {
Some(self.inline_completion_provider.as_ref()?.provider.clone()) 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( fn render_code_actions_indicator(
&self, &self,
_style: &EditorStyle, _style: &EditorStyle,
@ -5137,7 +5164,11 @@ impl Editor {
fn hide_context_menu(&mut self, cx: &mut ViewContext<Self>) -> Option<CodeContextMenu> { fn hide_context_menu(&mut self, cx: &mut ViewContext<Self>) -> Option<CodeContextMenu> {
cx.notify(); cx.notify();
self.completion_tasks.clear(); 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( fn show_snippet_choices(

View file

@ -35,6 +35,7 @@ pub struct EditorSettings {
pub auto_signature_help: bool, pub auto_signature_help: bool,
pub show_signature_help_after_edits: bool, pub show_signature_help_after_edits: bool,
pub jupyter: Jupyter, pub jupyter: Jupyter,
pub show_inline_completions_in_menu: bool,
} }
#[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)]
@ -300,6 +301,12 @@ pub struct EditorSettingsContent {
/// Default: false /// Default: false
pub show_signature_help_after_edits: Option<bool>, pub show_signature_help_after_edits: Option<bool>,
/// 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<bool>,
/// Jupyter REPL settings. /// Jupyter REPL settings.
pub jupyter: Option<JupyterContent>, pub jupyter: Option<JupyterContent>,
} }

View file

@ -321,6 +321,10 @@ impl InlineCompletionProvider for FakeInlineCompletionProvider {
"Fake Completion Provider" "Fake Completion Provider"
} }
fn show_completions_in_menu() -> bool {
false
}
fn is_enabled( fn is_enabled(
&self, &self,
_buffer: &gpui::Model<language::Buffer>, _buffer: &gpui::Model<language::Buffer>,

View file

@ -20,6 +20,7 @@ pub struct InlineCompletion {
pub trait InlineCompletionProvider: 'static + Sized { pub trait InlineCompletionProvider: 'static + Sized {
fn name() -> &'static str; fn name() -> &'static str;
fn display_name() -> &'static str; fn display_name() -> &'static str;
fn show_completions_in_menu() -> bool;
fn is_enabled( fn is_enabled(
&self, &self,
buffer: &Model<Buffer>, buffer: &Model<Buffer>,
@ -59,6 +60,7 @@ pub trait InlineCompletionProviderHandle {
cursor_position: language::Anchor, cursor_position: language::Anchor,
cx: &AppContext, cx: &AppContext,
) -> bool; ) -> bool;
fn show_completions_in_menu(&self) -> bool;
fn refresh( fn refresh(
&self, &self,
buffer: Model<Buffer>, buffer: Model<Buffer>,
@ -95,6 +97,10 @@ where
T::display_name() T::display_name()
} }
fn show_completions_in_menu(&self) -> bool {
T::show_completions_in_menu()
}
fn is_enabled( fn is_enabled(
&self, &self,
buffer: &Model<Buffer>, buffer: &Model<Buffer>,

View file

@ -102,6 +102,10 @@ impl InlineCompletionProvider for SupermavenCompletionProvider {
"Supermaven" "Supermaven"
} }
fn show_completions_in_menu() -> bool {
false
}
fn is_enabled(&self, buffer: &Model<Buffer>, cursor_position: Anchor, cx: &AppContext) -> bool { fn is_enabled(&self, buffer: &Model<Buffer>, cursor_position: Anchor, cx: &AppContext) -> bool {
if !self.supermaven.read(cx).is_enabled() { if !self.supermaven.read(cx).is_enabled() {
return false; return false;

View file

@ -932,6 +932,10 @@ impl inline_completion::InlineCompletionProvider for ZetaInlineCompletionProvide
"Zeta" "Zeta"
} }
fn show_completions_in_menu() -> bool {
true
}
fn is_enabled( fn is_enabled(
&self, &self,
buffer: &Model<Buffer>, buffer: &Model<Buffer>,