From a8b04369aebe0882733bf3d152288fd02b2612bc Mon Sep 17 00:00:00 2001 From: Andrew Farkas <6060305+HactarCE@users.noreply.github.com> Date: Mon, 10 Nov 2025 17:00:59 -0500 Subject: [PATCH] Refactor completions (#42122) This is progress toward multi-word snippets (including snippets with prefixes containing symbols) Release Notes: - Removed `trigger` argument in `ShowCompletions` command --------- Co-authored-by: Conrad Irwin --- crates/agent_ui/src/text_thread_editor.rs | 2 +- crates/editor/src/actions.rs | 11 +- crates/editor/src/code_context_menus.rs | 9 + crates/editor/src/editor.rs | 261 +++++++++++----------- crates/editor/src/editor_tests.rs | 65 ++++-- 5 files changed, 192 insertions(+), 156 deletions(-) diff --git a/crates/agent_ui/src/text_thread_editor.rs b/crates/agent_ui/src/text_thread_editor.rs index 19063075f9cf7382270c4dbaf4930596a7592676..e7f16b8886c719cf60763f651fe9abb9fe33d828 100644 --- a/crates/agent_ui/src/text_thread_editor.rs +++ b/crates/agent_ui/src/text_thread_editor.rs @@ -478,7 +478,7 @@ impl TextThreadEditor { editor.insert(&format!("/{name}"), window, cx); if command.accepts_arguments() { editor.insert(" ", window, cx); - editor.show_completions(&ShowCompletions::default(), window, cx); + editor.show_completions(&ShowCompletions, window, cx); } }); }); diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 276f20a7aacc9315f27a929876984342edc8d394..e823b06910fba67a38754ece6ad746f5f632e613 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -213,15 +213,6 @@ pub struct ExpandExcerptsDown { pub(super) lines: u32, } -/// Shows code completion suggestions at the cursor position. -#[derive(PartialEq, Clone, Deserialize, Default, JsonSchema, Action)] -#[action(namespace = editor)] -#[serde(deny_unknown_fields)] -pub struct ShowCompletions { - #[serde(default)] - pub(super) trigger: Option, -} - /// Handles text input in the editor. #[derive(PartialEq, Clone, Deserialize, Default, JsonSchema, Action)] #[action(namespace = editor)] @@ -736,6 +727,8 @@ actions!( SelectToStartOfParagraph, /// Extends selection up. SelectUp, + /// Shows code completion suggestions at the cursor position. + ShowCompletions, /// Shows the system character palette. ShowCharacterPalette, /// Shows edit prediction at cursor. diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index b7f3d57870a9504b7e6f9f736a0951b9b4b733e5..9e29cd955a80c7025ef2ff1ee5aaf38c665bed1a 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -252,8 +252,17 @@ enum MarkdownCacheKey { #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum CompletionsMenuSource { + /// Show all completions (words, snippets, LSP) Normal, + /// Show only snippets (not words or LSP) + /// + /// Used after typing a non-word character + SnippetsOnly, + /// Tab stops within a snippet that have a predefined finite set of choices SnippetChoices, + /// Show only words (not snippets or LSP) + /// + /// Used when word completions are explicitly triggered Words { ignore_threshold: bool }, } diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index 8c015d09c0717e2df52f8c5f85cead07be95bf50..8c165a6d7ce0a5410000cb21d9616e4c508a6fb3 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -3262,7 +3262,7 @@ impl Editor { }; if continue_showing { - self.show_completions(&ShowCompletions { trigger: None }, window, cx); + self.open_or_update_completions_menu(None, None, false, window, cx); } else { self.hide_context_menu(window, cx); } @@ -5097,57 +5097,18 @@ impl Editor { ignore_threshold: false, }), None, - window, - cx, - ); - } - Some(CompletionsMenuSource::Normal) - | Some(CompletionsMenuSource::SnippetChoices) - | None - if self.is_completion_trigger( - text, trigger_in_words, - completions_source.is_some(), - cx, - ) => - { - self.show_completions( - &ShowCompletions { - trigger: Some(text.to_owned()).filter(|x| !x.is_empty()), - }, window, cx, - ) - } - _ => { - self.hide_context_menu(window, cx); + ); } - } - } - - fn is_completion_trigger( - &self, - text: &str, - trigger_in_words: bool, - menu_is_open: bool, - cx: &mut Context, - ) -> bool { - let position = self.selections.newest_anchor().head(); - let Some(buffer) = self.buffer.read(cx).buffer_for_anchor(position, cx) else { - return false; - }; - - if let Some(completion_provider) = &self.completion_provider { - completion_provider.is_completion_trigger( - &buffer, - position.text_anchor, - text, - trigger_in_words, - menu_is_open, + _ => self.open_or_update_completions_menu( + None, + Some(text.to_owned()).filter(|x| !x.is_empty()), + true, + window, cx, - ) - } else { - false + ), } } @@ -5425,6 +5386,7 @@ impl Editor { ignore_threshold: true, }), None, + false, window, cx, ); @@ -5432,17 +5394,18 @@ impl Editor { pub fn show_completions( &mut self, - options: &ShowCompletions, + _: &ShowCompletions, window: &mut Window, cx: &mut Context, ) { - self.open_or_update_completions_menu(None, options.trigger.as_deref(), window, cx); + self.open_or_update_completions_menu(None, None, false, window, cx); } fn open_or_update_completions_menu( &mut self, requested_source: Option, - trigger: Option<&str>, + trigger: Option, + trigger_in_words: bool, window: &mut Window, cx: &mut Context, ) { @@ -5450,6 +5413,15 @@ impl Editor { return; } + let completions_source = self + .context_menu + .borrow() + .as_ref() + .and_then(|menu| match menu { + CodeContextMenu::Completions(completions_menu) => Some(completions_menu.source), + CodeContextMenu::CodeActions(_) => None, + }); + let multibuffer_snapshot = self.buffer.read(cx).read(cx); // Typically `start` == `end`, but with snippet tabstop choices the default choice is @@ -5497,7 +5469,8 @@ impl Editor { ignore_word_threshold = ignore_threshold; None } - Some(CompletionsMenuSource::SnippetChoices) => { + Some(CompletionsMenuSource::SnippetChoices) + | Some(CompletionsMenuSource::SnippetsOnly) => { log::error!("bug: SnippetChoices requested_source is not handled"); None } @@ -5511,13 +5484,19 @@ impl Editor { .as_ref() .is_none_or(|provider| provider.filter_completions()); + let was_snippets_only = matches!( + completions_source, + Some(CompletionsMenuSource::SnippetsOnly) + ); + if let Some(CodeContextMenu::Completions(menu)) = self.context_menu.borrow_mut().as_mut() { if filter_completions { menu.filter(query.clone(), provider.clone(), window, cx); } // When `is_incomplete` is false, no need to re-query completions when the current query // is a suffix of the initial query. - if !menu.is_incomplete { + let was_complete = !menu.is_incomplete; + if was_complete && !was_snippets_only { // If the new query is a suffix of the old query (typing more characters) and // the previous result was complete, the existing completions can be filtered. // @@ -5541,23 +5520,6 @@ impl Editor { } }; - let trigger_kind = match trigger { - Some(trigger) if buffer.read(cx).completion_triggers().contains(trigger) => { - CompletionTriggerKind::TRIGGER_CHARACTER - } - _ => CompletionTriggerKind::INVOKED, - }; - let completion_context = CompletionContext { - trigger_character: trigger.and_then(|trigger| { - if trigger_kind == CompletionTriggerKind::TRIGGER_CHARACTER { - Some(String::from(trigger)) - } else { - None - } - }), - trigger_kind, - }; - let Anchor { excerpt_id: buffer_excerpt_id, text_anchor: buffer_position, @@ -5610,54 +5572,88 @@ impl Editor { .as_ref() .is_none_or(|query| !query.chars().any(|c| c.is_digit(10))); - let omit_word_completions = !self.word_completions_enabled - || (!ignore_word_threshold - && match &query { - Some(query) => query.chars().count() < completion_settings.words_min_length, - None => completion_settings.words_min_length != 0, - }); - - let (mut words, provider_responses) = match &provider { - Some(provider) => { - let provider_responses = provider.completions( - buffer_excerpt_id, + let load_provider_completions = provider.as_ref().is_some_and(|provider| { + trigger.as_ref().is_none_or(|trigger| { + provider.is_completion_trigger( &buffer, - buffer_position, - completion_context, - window, + position.text_anchor, + trigger, + trigger_in_words, + completions_source.is_some(), cx, - ); + ) + }) + }); - let words = match (omit_word_completions, completion_settings.words) { - (true, _) | (_, WordsCompletionMode::Disabled) => { - Task::ready(BTreeMap::default()) - } - (false, WordsCompletionMode::Enabled | WordsCompletionMode::Fallback) => cx - .background_spawn(async move { - buffer_snapshot.words_in_range(WordsQuery { - fuzzy_contents: None, - range: word_search_range, - skip_digits, - }) - }), - }; + let provider_responses = if let Some(provider) = &provider + && load_provider_completions + { + let trigger_character = + trigger.filter(|trigger| buffer.read(cx).completion_triggers().contains(trigger)); + let completion_context = CompletionContext { + trigger_kind: match &trigger_character { + Some(_) => CompletionTriggerKind::TRIGGER_CHARACTER, + None => CompletionTriggerKind::INVOKED, + }, + trigger_character, + }; - (words, provider_responses) - } - None => { - let words = if omit_word_completions { - Task::ready(BTreeMap::default()) - } else { - cx.background_spawn(async move { - buffer_snapshot.words_in_range(WordsQuery { - fuzzy_contents: None, - range: word_search_range, - skip_digits, - }) - }) - }; - (words, Task::ready(Ok(Vec::new()))) - } + provider.completions( + buffer_excerpt_id, + &buffer, + buffer_position, + completion_context, + window, + cx, + ) + } else { + Task::ready(Ok(Vec::new())) + }; + + let load_word_completions = if !self.word_completions_enabled { + false + } else if requested_source + == Some(CompletionsMenuSource::Words { + ignore_threshold: true, + }) + { + true + } else { + load_provider_completions + && completion_settings.words != WordsCompletionMode::Disabled + && (ignore_word_threshold || { + let words_min_length = completion_settings.words_min_length; + // check whether word has at least `words_min_length` characters + let query_chars = query.iter().flat_map(|q| q.chars()); + query_chars.take(words_min_length).count() == words_min_length + }) + }; + + let mut words = if load_word_completions { + cx.background_spawn(async move { + buffer_snapshot.words_in_range(WordsQuery { + fuzzy_contents: None, + range: word_search_range, + skip_digits, + }) + }) + } else { + Task::ready(BTreeMap::default()) + }; + + let snippets = if let Some(provider) = &provider + && provider.show_snippets() + && let Some(project) = self.project() + { + project.update(cx, |project, cx| { + snippet_completions(project, &buffer, buffer_position, cx) + }) + } else { + Task::ready(Ok(CompletionResponse { + completions: Vec::new(), + display_options: Default::default(), + is_incomplete: false, + })) }; let snippet_sort_order = EditorSettings::get_global(cx).snippet_sort_order; @@ -5715,6 +5711,13 @@ impl Editor { confirm: None, })); + completions.extend( + snippets + .await + .into_iter() + .flat_map(|response| response.completions), + ); + let menu = if completions.is_empty() { None } else { @@ -5726,7 +5729,11 @@ impl Editor { .map(|workspace| workspace.read(cx).app_state().languages.clone()); let menu = CompletionsMenu::new( id, - requested_source.unwrap_or(CompletionsMenuSource::Normal), + requested_source.unwrap_or(if load_provider_completions { + CompletionsMenuSource::Normal + } else { + CompletionsMenuSource::SnippetsOnly + }), sort_completions, show_completion_documentation, position, @@ -6056,7 +6063,7 @@ impl Editor { .as_ref() .is_some_and(|confirm| confirm(intent, window, cx)); if show_new_completions_on_confirm { - self.show_completions(&ShowCompletions { trigger: None }, window, cx); + self.open_or_update_completions_menu(None, None, false, window, cx); } let provider = self.completion_provider.as_ref()?; @@ -12852,6 +12859,10 @@ impl Editor { }); } + // 🤔 | .. | show_in_menu | + // | .. | true true + // | had_edit_prediction | false true + let trigger_in_words = this.show_edit_predictions_in_menu() || !had_active_edit_prediction; @@ -23059,6 +23070,10 @@ pub trait CompletionProvider { fn filter_completions(&self) -> bool { true } + + fn show_snippets(&self) -> bool { + false + } } pub trait CodeActionProvider { @@ -23319,16 +23334,8 @@ impl CompletionProvider for Entity { cx: &mut Context, ) -> Task>> { self.update(cx, |project, cx| { - let snippets = snippet_completions(project, buffer, buffer_position, cx); - let project_completions = project.completions(buffer, buffer_position, options, cx); - cx.background_spawn(async move { - let mut responses = project_completions.await?; - let snippets = snippets.await?; - if !snippets.completions.is_empty() { - responses.push(snippets); - } - Ok(responses) - }) + let task = project.completions(buffer, buffer_position, options, cx); + cx.background_spawn(task) }) } @@ -23400,6 +23407,10 @@ impl CompletionProvider for Entity { buffer.completion_triggers().contains(text) } + + fn show_snippets(&self) -> bool { + true + } } impl SemanticsProvider for Entity { diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index c1fbc9053882d9e6a74e27a8cd7fb788289d1fa7..ce97cf9a1cc68ed4ff06d57ac02e0dbb9fdd8788 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -13827,7 +13827,7 @@ async fn test_completion_mode(cx: &mut TestAppContext) { cx.set_state(&run.initial_state); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); let counter = Arc::new(AtomicUsize::new(0)); @@ -13887,7 +13887,7 @@ async fn test_completion_with_mode_specified_by_action(cx: &mut TestAppContext) cx.set_state(initial_state); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); let counter = Arc::new(AtomicUsize::new(0)); @@ -13923,7 +13923,7 @@ async fn test_completion_with_mode_specified_by_action(cx: &mut TestAppContext) cx.set_state(initial_state); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); handle_completion_request_with_insert_and_replace( &mut cx, @@ -14010,7 +14010,7 @@ async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut T "}; cx.set_state(initial_state); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); handle_completion_request_with_insert_and_replace( &mut cx, @@ -14064,7 +14064,7 @@ async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut T "}; cx.set_state(initial_state); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); handle_completion_request_with_insert_and_replace( &mut cx, @@ -14113,7 +14113,7 @@ async fn test_completion_replacing_surrounding_text_with_multicursors(cx: &mut T "}; cx.set_state(initial_state); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); handle_completion_request_with_insert_and_replace( &mut cx, @@ -14264,7 +14264,7 @@ async fn test_completion_in_multibuffer_with_replace_range(cx: &mut TestAppConte }); editor.update_in(cx, |editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); fake_server @@ -14503,7 +14503,7 @@ async fn test_completion(cx: &mut TestAppContext) { cx.assert_editor_state("editor.cloˇ"); assert!(cx.editor(|e, _, _| e.context_menu.borrow_mut().is_none())); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); handle_completion_request( "editor.", @@ -14902,7 +14902,7 @@ async fn test_word_completions_usually_skip_digits(cx: &mut TestAppContext) { 4.5f32 "}); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions::default(), window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); cx.executor().run_until_parked(); cx.condition(|editor, _| editor.context_menu_visible()) @@ -14928,7 +14928,7 @@ async fn test_word_completions_usually_skip_digits(cx: &mut TestAppContext) { 33.35f32 "}); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions::default(), window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); cx.executor().run_until_parked(); cx.condition(|editor, _| editor.context_menu_visible()) @@ -15056,6 +15056,35 @@ async fn test_word_completions_disabled(cx: &mut TestAppContext) { }); } +#[gpui::test] +async fn test_word_completions_disabled_with_no_provider(cx: &mut TestAppContext) { + init_test(cx, |language_settings| { + language_settings.defaults.completions = Some(CompletionSettingsContent { + words: Some(WordsCompletionMode::Disabled), + words_min_length: Some(0), + lsp_insert_mode: Some(LspInsertMode::Insert), + ..Default::default() + }); + }); + + let mut cx = EditorLspTestContext::new_rust(lsp::ServerCapabilities::default(), cx).await; + cx.update_editor(|editor, _, _| { + editor.set_completion_provider(None); + }); + cx.set_state(indoc! {"ˇ + wow + wowen + wowser + "}); + cx.simulate_keystroke("w"); + cx.executor().run_until_parked(); + cx.update_editor(|editor, _, _| { + if editor.context_menu.borrow_mut().is_some() { + panic!("expected completion menu to be hidden, as disabled in settings"); + } + }); +} + fn gen_text_edit(params: &CompletionParams, text: &str) -> Option { let position = || lsp::Position { line: params.text_document_position.position.line, @@ -15352,13 +15381,7 @@ async fn test_as_is_completions(cx: &mut TestAppContext) { cx.set_state("fn a() {}\n nˇ"); cx.executor().run_until_parked(); cx.update_editor(|editor, window, cx| { - editor.show_completions( - &ShowCompletions { - trigger: Some("\n".into()), - }, - window, - cx, - ); + editor.trigger_completion_on_input("n", true, window, cx) }); cx.executor().run_until_parked(); @@ -15456,7 +15479,7 @@ int fn_branch(bool do_branch1, bool do_branch2); }))) }); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); cx.executor().run_until_parked(); cx.update_editor(|editor, window, cx| { @@ -15505,7 +15528,7 @@ int fn_branch(bool do_branch1, bool do_branch2); }))) }); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); cx.executor().run_until_parked(); cx.update_editor(|editor, window, cx| { @@ -17995,7 +18018,7 @@ async fn test_context_menus_hide_hover_popover(cx: &mut gpui::TestAppContext) { } }); cx.update_editor(|editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); completion_requests.next().await; cx.condition(|editor, _| editor.context_menu_visible()) @@ -24391,7 +24414,7 @@ async fn test_html_linked_edits_on_completion(cx: &mut TestAppContext) { ]))) }); editor.update_in(cx, |editor, window, cx| { - editor.show_completions(&ShowCompletions { trigger: None }, window, cx); + editor.show_completions(&ShowCompletions, window, cx); }); cx.run_until_parked(); completion_handle.next().await.unwrap();