From 566c5f91a7ee4dffe8d3afa8c4ff3b04e71f21eb Mon Sep 17 00:00:00 2001 From: Kirill Bulatov Date: Fri, 14 Mar 2025 17:18:55 +0200 Subject: [PATCH] Refine word completions (#26779) Follow-up of https://github.com/zed-industries/zed/pull/26410 * Extract word completions into their own, `editor::ShowWordCompletions` action so those could be triggered independently of completions * Assign `ctrl-shift-space` binding to this new action * Still keep words returned along the completions as in the original PR, but: * Tone down regular completions' fallback logic, skip words when the language server responds with empty list of completions, but keep on adding words if nothing or an error were returned instead * Adjust the defaults to wait for LSP completions infinitely * Skip "words" with digits such as `0_usize` or `2.f32` from completion items, unless a completion query has digits in it Release Notes: - N/A --- assets/keymaps/default-linux.json | 1 + assets/keymaps/default-macos.json | 1 + assets/settings/default.json | 9 +- .../src/slash_command.rs | 208 +++++++++--------- .../src/chat_panel/message_editor.rs | 44 ++-- crates/editor/src/actions.rs | 5 +- crates/editor/src/editor.rs | 172 +++++++++------ crates/editor/src/editor_tests.rs | 61 +++++ crates/editor/src/element.rs | 1 + .../src/extension_store_test.rs | 1 + crates/language/src/buffer.rs | 47 ++-- crates/language/src/buffer_tests.rs | 73 +++++- crates/language/src/language_settings.rs | 13 +- crates/multi_buffer/src/multi_buffer.rs | 8 - crates/project/src/lsp_store.rs | 24 +- crates/project/src/project.rs | 2 +- crates/project/src/project_tests.rs | 6 +- .../remote_server/src/remote_editing_tests.rs | 6 +- 18 files changed, 431 insertions(+), 251 deletions(-) diff --git a/assets/keymaps/default-linux.json b/assets/keymaps/default-linux.json index 5c5ef39708..99e24e679a 100644 --- a/assets/keymaps/default-linux.json +++ b/assets/keymaps/default-linux.json @@ -362,6 +362,7 @@ "ctrl-k ctrl-0": "editor::FoldAll", "ctrl-k ctrl-j": "editor::UnfoldAll", "ctrl-space": "editor::ShowCompletions", + "ctrl-shift-space": "editor::ShowWordCompletions", "ctrl-.": "editor::ToggleCodeActions", "ctrl-k r": "editor::RevealInFileManager", "ctrl-k p": "editor::CopyPath", diff --git a/assets/keymaps/default-macos.json b/assets/keymaps/default-macos.json index 608ec76f85..4252351a79 100644 --- a/assets/keymaps/default-macos.json +++ b/assets/keymaps/default-macos.json @@ -466,6 +466,7 @@ // Using `ctrl-space` in Zed requires disabling the macOS global shortcut. // System Preferences->Keyboard->Keyboard Shortcuts->Input Sources->Select the previous input source (uncheck) "ctrl-space": "editor::ShowCompletions", + "ctrl-shift-space": "editor::ShowWordCompletions", "cmd-.": "editor::ToggleCodeActions", "cmd-k r": "editor::RevealInFileManager", "cmd-k p": "editor::CopyPath", diff --git a/assets/settings/default.json b/assets/settings/default.json index d883953740..cabef3c9ca 100644 --- a/assets/settings/default.json +++ b/assets/settings/default.json @@ -1092,11 +1092,12 @@ // // May take 3 values: // 1. "enabled" - // Always fetch document's words for completions. + // Always fetch document's words for completions along with LSP completions. // 2. "fallback" - // Only if LSP response errors/times out/is empty, use document's words to show completions. + // Only if LSP response errors or times out, use document's words to show completions. // 3. "disabled" // Never fetch or complete document's words for completions. + // (Word-based completions can still be queried via a separate action) // // Default: fallback "words": "fallback", @@ -1107,8 +1108,8 @@ // When fetching LSP completions, determines how long to wait for a response of a particular server. // When set to 0, waits indefinitely. // - // Default: 500 - "lsp_fetch_timeout_ms": 500 + // Default: 0 + "lsp_fetch_timeout_ms": 0 }, // Different settings for specific languages. "languages": { diff --git a/crates/assistant_context_editor/src/slash_command.rs b/crates/assistant_context_editor/src/slash_command.rs index 4ca1a9fd78..8b7a745e56 100644 --- a/crates/assistant_context_editor/src/slash_command.rs +++ b/crates/assistant_context_editor/src/slash_command.rs @@ -48,7 +48,7 @@ impl SlashCommandCompletionProvider { name_range: Range, window: &mut Window, cx: &mut App, - ) -> Task>> { + ) -> Task>>> { let slash_commands = self.slash_commands.clone(); let candidates = slash_commands .command_names(cx) @@ -71,65 +71,67 @@ impl SlashCommandCompletionProvider { .await; cx.update(|_, cx| { - matches - .into_iter() - .filter_map(|mat| { - let command = slash_commands.command(&mat.string, cx)?; - let mut new_text = mat.string.clone(); - let requires_argument = command.requires_argument(); - let accepts_arguments = command.accepts_arguments(); - if requires_argument || accepts_arguments { - new_text.push(' '); - } + Some( + matches + .into_iter() + .filter_map(|mat| { + let command = slash_commands.command(&mat.string, cx)?; + let mut new_text = mat.string.clone(); + let requires_argument = command.requires_argument(); + let accepts_arguments = command.accepts_arguments(); + if requires_argument || accepts_arguments { + new_text.push(' '); + } - let confirm = - editor - .clone() - .zip(workspace.clone()) - .map(|(editor, workspace)| { - let command_name = mat.string.clone(); - let command_range = command_range.clone(); - let editor = editor.clone(); - let workspace = workspace.clone(); - Arc::new( - move |intent: CompletionIntent, - window: &mut Window, - cx: &mut App| { - if !requires_argument - && (!accepts_arguments || intent.is_complete()) - { - editor - .update(cx, |editor, cx| { - editor.run_command( - command_range.clone(), - &command_name, - &[], - true, - workspace.clone(), - window, - cx, - ); - }) - .ok(); - false - } else { - requires_argument || accepts_arguments - } - }, - ) as Arc<_> - }); - Some(project::Completion { - old_range: name_range.clone(), - documentation: Some(CompletionDocumentation::SingleLine( - command.description().into(), - )), - new_text, - label: command.label(cx), - confirm, - source: CompletionSource::Custom, + let confirm = + editor + .clone() + .zip(workspace.clone()) + .map(|(editor, workspace)| { + let command_name = mat.string.clone(); + let command_range = command_range.clone(); + let editor = editor.clone(); + let workspace = workspace.clone(); + Arc::new( + move |intent: CompletionIntent, + window: &mut Window, + cx: &mut App| { + if !requires_argument + && (!accepts_arguments || intent.is_complete()) + { + editor + .update(cx, |editor, cx| { + editor.run_command( + command_range.clone(), + &command_name, + &[], + true, + workspace.clone(), + window, + cx, + ); + }) + .ok(); + false + } else { + requires_argument || accepts_arguments + } + }, + ) as Arc<_> + }); + Some(project::Completion { + old_range: name_range.clone(), + documentation: Some(CompletionDocumentation::SingleLine( + command.description().into(), + )), + new_text, + label: command.label(cx), + confirm, + source: CompletionSource::Custom, + }) }) - }) - .collect() + .collect(), + ) }) }) } @@ -143,7 +145,7 @@ impl SlashCommandCompletionProvider { last_argument_range: Range, window: &mut Window, cx: &mut App, - ) -> Task>> { + ) -> Task>>> { let new_cancel_flag = Arc::new(AtomicBool::new(false)); let mut flag = self.cancel_flag.lock(); flag.store(true, SeqCst); @@ -161,27 +163,28 @@ impl SlashCommandCompletionProvider { let workspace = self.workspace.clone(); let arguments = arguments.to_vec(); cx.background_spawn(async move { - Ok(completions - .await? - .into_iter() - .map(|new_argument| { - let confirm = - editor - .clone() - .zip(workspace.clone()) - .map(|(editor, workspace)| { - Arc::new({ - let mut completed_arguments = arguments.clone(); - if new_argument.replace_previous_arguments { - completed_arguments.clear(); - } else { - completed_arguments.pop(); - } - completed_arguments.push(new_argument.new_text.clone()); + Ok(Some( + completions + .await? + .into_iter() + .map(|new_argument| { + let confirm = + editor + .clone() + .zip(workspace.clone()) + .map(|(editor, workspace)| { + Arc::new({ + let mut completed_arguments = arguments.clone(); + if new_argument.replace_previous_arguments { + completed_arguments.clear(); + } else { + completed_arguments.pop(); + } + completed_arguments.push(new_argument.new_text.clone()); - let command_range = command_range.clone(); - let command_name = command_name.clone(); - move |intent: CompletionIntent, + let command_range = command_range.clone(); + let command_name = command_name.clone(); + move |intent: CompletionIntent, window: &mut Window, cx: &mut App| { if new_argument.after_completion.run() @@ -205,31 +208,32 @@ impl SlashCommandCompletionProvider { !new_argument.after_completion.run() } } - }) as Arc<_> - }); + }) as Arc<_> + }); - let mut new_text = new_argument.new_text.clone(); - if new_argument.after_completion == AfterCompletion::Continue { - new_text.push(' '); - } + let mut new_text = new_argument.new_text.clone(); + if new_argument.after_completion == AfterCompletion::Continue { + new_text.push(' '); + } - project::Completion { - old_range: if new_argument.replace_previous_arguments { - argument_range.clone() - } else { - last_argument_range.clone() - }, - label: new_argument.label, - new_text, - documentation: None, - confirm, - source: CompletionSource::Custom, - } - }) - .collect()) + project::Completion { + old_range: if new_argument.replace_previous_arguments { + argument_range.clone() + } else { + last_argument_range.clone() + }, + label: new_argument.label, + new_text, + documentation: None, + confirm, + source: CompletionSource::Custom, + } + }) + .collect(), + )) }) } else { - Task::ready(Ok(Vec::new())) + Task::ready(Ok(Some(Vec::new()))) } } } @@ -242,7 +246,7 @@ impl CompletionProvider for SlashCommandCompletionProvider { _: editor::CompletionContext, window: &mut Window, cx: &mut Context, - ) -> Task>> { + ) -> Task>>> { let Some((name, arguments, command_range, last_argument_range)) = buffer.update(cx, |buffer, _cx| { let position = buffer_position.to_point(buffer); @@ -286,7 +290,7 @@ impl CompletionProvider for SlashCommandCompletionProvider { Some((name, arguments, command_range, last_argument_range)) }) else { - return Task::ready(Ok(Vec::new())); + return Task::ready(Ok(Some(Vec::new()))); }; if let Some((arguments, argument_range)) = arguments { diff --git a/crates/collab_ui/src/chat_panel/message_editor.rs b/crates/collab_ui/src/chat_panel/message_editor.rs index 49b2547175..760f4119c7 100644 --- a/crates/collab_ui/src/chat_panel/message_editor.rs +++ b/crates/collab_ui/src/chat_panel/message_editor.rs @@ -61,9 +61,9 @@ impl CompletionProvider for MessageEditorCompletionProvider { _: editor::CompletionContext, _window: &mut Window, cx: &mut Context, - ) -> Task>> { + ) -> Task>>> { let Some(handle) = self.0.upgrade() else { - return Task::ready(Ok(Vec::new())); + return Task::ready(Ok(None)); }; handle.update(cx, |message_editor, cx| { message_editor.completions(buffer, buffer_position, cx) @@ -246,20 +246,22 @@ impl MessageEditor { buffer: &Entity, end_anchor: Anchor, cx: &mut Context, - ) -> Task>> { + ) -> Task>>> { if let Some((start_anchor, query, candidates)) = self.collect_mention_candidates(buffer, end_anchor, cx) { if !candidates.is_empty() { return cx.spawn(|_, cx| async move { - Ok(Self::resolve_completions_for_candidates( - &cx, - query.as_str(), - &candidates, - start_anchor..end_anchor, - Self::completion_for_mention, - ) - .await) + Ok(Some( + Self::resolve_completions_for_candidates( + &cx, + query.as_str(), + &candidates, + start_anchor..end_anchor, + Self::completion_for_mention, + ) + .await, + )) }); } } @@ -269,19 +271,21 @@ impl MessageEditor { { if !candidates.is_empty() { return cx.spawn(|_, cx| async move { - Ok(Self::resolve_completions_for_candidates( - &cx, - query.as_str(), - candidates, - start_anchor..end_anchor, - Self::completion_for_emoji, - ) - .await) + Ok(Some( + Self::resolve_completions_for_candidates( + &cx, + query.as_str(), + candidates, + start_anchor..end_anchor, + Self::completion_for_emoji, + ) + .await, + )) }); } } - Task::ready(Ok(vec![])) + Task::ready(Ok(Some(Vec::new()))) } async fn resolve_completions_for_candidates( diff --git a/crates/editor/src/actions.rs b/crates/editor/src/actions.rs index 8db29cdd08..782e910b64 100644 --- a/crates/editor/src/actions.rs +++ b/crates/editor/src/actions.rs @@ -1,6 +1,6 @@ //! This module contains all actions supported by [`Editor`]. use super::*; -use gpui::{action_as, action_with_deprecated_aliases}; +use gpui::{action_as, action_with_deprecated_aliases, actions}; use schemars::JsonSchema; use util::serde::default_true; #[derive(PartialEq, Clone, Deserialize, Default, JsonSchema)] @@ -248,7 +248,7 @@ impl_actions!( ] ); -gpui::actions!( +actions!( editor, [ AcceptEditPrediction, @@ -404,6 +404,7 @@ gpui::actions!( ShowCharacterPalette, ShowEditPrediction, ShowSignatureHelp, + ShowWordCompletions, ShuffleLines, SortLinesCaseInsensitive, SortLinesCaseSensitive, diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index c043491f00..34b25dcbea 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -106,7 +106,7 @@ use language::{ point_from_lsp, text_diff_with_options, AutoindentMode, BracketMatch, BracketPair, Buffer, Capability, CharKind, CodeLabel, CursorShape, Diagnostic, DiffOptions, EditPredictionsMode, EditPreview, HighlightedText, IndentKind, IndentSize, Language, OffsetRangeExt, Point, - Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions, + Selection, SelectionGoal, TextObject, TransactionId, TreeSitterOptions, WordsQuery, }; use language::{point_to_lsp, BufferRow, CharClassifier, Runnable, RunnableRange}; use linked_editing_ranges::refresh_linked_ranges; @@ -3977,20 +3977,34 @@ impl Editor { })) } + pub fn show_word_completions( + &mut self, + _: &ShowWordCompletions, + window: &mut Window, + cx: &mut Context, + ) { + self.open_completions_menu(true, None, window, cx); + } + pub fn show_completions( &mut self, options: &ShowCompletions, window: &mut Window, cx: &mut Context, + ) { + self.open_completions_menu(false, options.trigger.as_deref(), window, cx); + } + + fn open_completions_menu( + &mut self, + ignore_completion_provider: bool, + trigger: Option<&str>, + window: &mut Window, + cx: &mut Context, ) { if self.pending_rename.is_some() { return; } - - let Some(provider) = self.completion_provider.as_ref() else { - return; - }; - if !self.snippet_stack.is_empty() && self.context_menu.borrow().as_ref().is_some() { return; } @@ -4012,14 +4026,14 @@ impl Editor { let query = Self::completion_query(&self.buffer.read(cx).read(cx), position); - let trigger_kind = match &options.trigger { + 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: options.trigger.as_ref().and_then(|trigger| { + trigger_character: trigger.and_then(|trigger| { if trigger_kind == CompletionTriggerKind::TRIGGER_CHARACTER { Some(String::from(trigger)) } else { @@ -4028,8 +4042,7 @@ impl Editor { }), trigger_kind, }; - let completions = - provider.completions(&buffer, buffer_position, completion_context, window, cx); + let (old_range, word_kind) = buffer_snapshot.surrounding_word(buffer_position); let (old_range, word_to_exclude) = if word_kind == Some(CharKind::Word) { let word_to_exclude = buffer_snapshot @@ -4067,15 +4080,49 @@ impl Editor { ); let word_search_range = buffer_snapshot.point_to_offset(min_word_search) ..buffer_snapshot.point_to_offset(max_word_search); - let words = match completion_settings.words { - WordsCompletionMode::Disabled => Task::ready(HashMap::default()), - WordsCompletionMode::Enabled | WordsCompletionMode::Fallback => { - cx.background_spawn(async move { - buffer_snapshot.words_in_range(None, word_search_range) - }) + + let provider = self + .completion_provider + .as_ref() + .filter(|_| !ignore_completion_provider); + let skip_digits = query + .as_ref() + .map_or(true, |query| !query.chars().any(|c| c.is_digit(10))); + + let (mut words, provided_completions) = match provider { + Some(provider) => { + let completions = + provider.completions(&buffer, buffer_position, completion_context, window, cx); + + let words = match completion_settings.words { + WordsCompletionMode::Disabled => Task::ready(HashMap::default()), + WordsCompletionMode::Enabled | WordsCompletionMode::Fallback => cx + .background_spawn(async move { + buffer_snapshot.words_in_range(WordsQuery { + fuzzy_contents: None, + range: word_search_range, + skip_digits, + }) + }), + }; + + (words, completions) } + None => ( + cx.background_spawn(async move { + buffer_snapshot.words_in_range(WordsQuery { + fuzzy_contents: None, + range: word_search_range, + skip_digits, + }) + }), + Task::ready(Ok(None)), + ), }; - let sort_completions = provider.sort_completions(); + + let sort_completions = provider + .as_ref() + .map_or(true, |provider| provider.sort_completions()); let id = post_inc(&mut self.next_completion_id); let task = cx.spawn_in(window, |editor, mut cx| { @@ -4083,55 +4130,34 @@ impl Editor { editor.update(&mut cx, |this, _| { this.completion_tasks.retain(|(task_id, _)| *task_id >= id); })?; - let mut completions = completions.await.log_err().unwrap_or_default(); - match completion_settings.words { - WordsCompletionMode::Enabled => { - let mut words = words.await; - if let Some(word_to_exclude) = &word_to_exclude { - words.remove(word_to_exclude); - } - for lsp_completion in &completions { - words.remove(&lsp_completion.new_text); - } - completions.extend(words.into_iter().map(|(word, word_range)| { - Completion { - old_range: old_range.clone(), - new_text: word.clone(), - label: CodeLabel::plain(word, None), - documentation: None, - source: CompletionSource::BufferWord { - word_range, - resolved: false, - }, - confirm: None, - } - })); + let mut completions = Vec::new(); + if let Some(provided_completions) = provided_completions.await.log_err().flatten() { + completions.extend(provided_completions); + if completion_settings.words == WordsCompletionMode::Fallback { + words = Task::ready(HashMap::default()); } - WordsCompletionMode::Fallback => { - if completions.is_empty() { - completions.extend( - words - .await - .into_iter() - .filter(|(word, _)| word_to_exclude.as_ref() != Some(word)) - .map(|(word, word_range)| Completion { - old_range: old_range.clone(), - new_text: word.clone(), - label: CodeLabel::plain(word, None), - documentation: None, - source: CompletionSource::BufferWord { - word_range, - resolved: false, - }, - confirm: None, - }), - ); - } - } - WordsCompletionMode::Disabled => {} } + let mut words = words.await; + if let Some(word_to_exclude) = &word_to_exclude { + words.remove(word_to_exclude); + } + for lsp_completion in &completions { + words.remove(&lsp_completion.new_text); + } + completions.extend(words.into_iter().map(|(word, word_range)| Completion { + old_range: old_range.clone(), + new_text: word.clone(), + label: CodeLabel::plain(word, None), + documentation: None, + source: CompletionSource::BufferWord { + word_range, + resolved: false, + }, + confirm: None, + })); + let menu = if completions.is_empty() { None } else { @@ -4188,7 +4214,7 @@ impl Editor { } })?; - Ok::<_, anyhow::Error>(()) + anyhow::Ok(()) } .log_err() }); @@ -16913,7 +16939,7 @@ pub trait CompletionProvider { trigger: CompletionContext, window: &mut Window, cx: &mut Context, - ) -> Task>>; + ) -> Task>>>; fn resolve_completions( &self, @@ -17153,15 +17179,25 @@ impl CompletionProvider for Entity { options: CompletionContext, _window: &mut Window, cx: &mut Context, - ) -> Task>> { + ) -> 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 completions = project_completions.await?; let snippets_completions = snippets.await?; - completions.extend(snippets_completions); - Ok(completions) + match project_completions.await? { + Some(mut completions) => { + completions.extend(snippets_completions); + Ok(Some(completions)) + } + None => { + if snippets_completions.is_empty() { + Ok(None) + } else { + Ok(Some(snippets_completions)) + } + } + } }) }) } diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 90c9842359..c2cad6528d 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -9352,6 +9352,67 @@ async fn test_word_completions_do_not_duplicate_lsp_ones(cx: &mut TestAppContext }); } +#[gpui::test] +async fn test_word_completions_usually_skip_digits(cx: &mut TestAppContext) { + init_test(cx, |language_settings| { + language_settings.defaults.completions = Some(CompletionSettings { + words: WordsCompletionMode::Fallback, + lsp: false, + lsp_fetch_timeout_ms: 0, + }); + }); + + let mut cx = EditorLspTestContext::new_rust(lsp::ServerCapabilities::default(), cx).await; + + cx.set_state(indoc! {"ˇ + 0_usize + let + 33 + 4.5f32 + "}); + cx.update_editor(|editor, window, cx| { + editor.show_completions(&ShowCompletions::default(), window, cx); + }); + cx.executor().run_until_parked(); + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + cx.update_editor(|editor, window, cx| { + if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() + { + assert_eq!( + completion_menu_entries(&menu), + &["let"], + "With no digits in the completion query, no digits should be in the word completions" + ); + } else { + panic!("expected completion menu to be open"); + } + editor.cancel(&Cancel, window, cx); + }); + + cx.set_state(indoc! {"3ˇ + 0_usize + let + 3 + 33.35f32 + "}); + cx.update_editor(|editor, window, cx| { + editor.show_completions(&ShowCompletions::default(), window, cx); + }); + cx.executor().run_until_parked(); + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + cx.update_editor(|editor, _, _| { + if let Some(CodeContextMenu::Completions(menu)) = editor.context_menu.borrow_mut().as_ref() + { + assert_eq!(completion_menu_entries(&menu), &["33", "35f32"], "The digit is in the completion query, \ + return matching words with digits (`33`, `35f32`) but exclude query duplicates (`3`)"); + } else { + panic!("expected completion menu to be open"); + } + }); +} + #[gpui::test] async fn test_multiline_completion(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/editor/src/element.rs b/crates/editor/src/element.rs index b55f656dc6..812cf3c143 100644 --- a/crates/editor/src/element.rs +++ b/crates/editor/src/element.rs @@ -390,6 +390,7 @@ impl EditorElement { register_action(editor, window, Editor::set_mark); register_action(editor, window, Editor::swap_selection_ends); register_action(editor, window, Editor::show_completions); + register_action(editor, window, Editor::show_word_completions); register_action(editor, window, Editor::toggle_code_actions); register_action(editor, window, Editor::open_excerpts); register_action(editor, window, Editor::open_excerpts_in_split); diff --git a/crates/extension_host/src/extension_store_test.rs b/crates/extension_host/src/extension_store_test.rs index ad28676b37..9e3fc4a175 100644 --- a/crates/extension_host/src/extension_store_test.rs +++ b/crates/extension_host/src/extension_store_test.rs @@ -714,6 +714,7 @@ async fn test_extension_store_with_test_extension(cx: &mut TestAppContext) { }) .await .unwrap() + .unwrap() .into_iter() .map(|c| c.label.text) .collect::>(); diff --git a/crates/language/src/buffer.rs b/crates/language/src/buffer.rs index 91ad627cbe..34632fa26b 100644 --- a/crates/language/src/buffer.rs +++ b/crates/language/src/buffer.rs @@ -4146,12 +4146,9 @@ impl BufferSnapshot { } } - pub fn words_in_range( - &self, - query: Option<&str>, - range: Range, - ) -> HashMap> { - if query.map_or(false, |query| query.is_empty()) { + pub fn words_in_range(&self, query: WordsQuery) -> HashMap> { + let query_str = query.fuzzy_contents; + if query_str.map_or(false, |query| query.is_empty()) { return HashMap::default(); } @@ -4161,13 +4158,13 @@ impl BufferSnapshot { })); let mut query_ix = 0; - let query = query.map(|query| query.chars().collect::>()); - let query_len = query.as_ref().map_or(0, |query| query.len()); + let query_chars = query_str.map(|query| query.chars().collect::>()); + let query_len = query_chars.as_ref().map_or(0, |query| query.len()); let mut words = HashMap::default(); let mut current_word_start_ix = None; - let mut chunk_ix = range.start; - for chunk in self.chunks(range, false) { + let mut chunk_ix = query.range.start; + for chunk in self.chunks(query.range, false) { for (i, c) in chunk.text.char_indices() { let ix = chunk_ix + i; if classifier.is_word(c) { @@ -4175,12 +4172,9 @@ impl BufferSnapshot { current_word_start_ix = Some(ix); } - if let Some(query) = &query { + if let Some(query_chars) = &query_chars { if query_ix < query_len { - let query_c = query.get(query_ix).expect( - "query_ix is a vec of chars, which we access only if before the end", - ); - if c.to_lowercase().eq(query_c.to_lowercase()) { + if c.to_lowercase().eq(query_chars[query_ix].to_lowercase()) { query_ix += 1; } } @@ -4189,10 +4183,16 @@ impl BufferSnapshot { } else if let Some(word_start) = current_word_start_ix.take() { if query_ix == query_len { let word_range = self.anchor_before(word_start)..self.anchor_after(ix); - words.insert( - self.text_for_range(word_start..ix).collect::(), - word_range, - ); + let mut word_text = self.text_for_range(word_start..ix).peekable(); + let first_char = word_text + .peek() + .and_then(|first_chunk| first_chunk.chars().next()); + // Skip empty and "words" starting with digits as a heuristic to reduce useless completions + if !query.skip_digits + || first_char.map_or(true, |first_char| !first_char.is_digit(10)) + { + words.insert(word_text.collect(), word_range); + } } } query_ix = 0; @@ -4204,6 +4204,15 @@ impl BufferSnapshot { } } +pub struct WordsQuery<'a> { + /// Only returns words with all chars from the fuzzy string in them. + pub fuzzy_contents: Option<&'a str>, + /// Skips words that start with a digit. + pub skip_digits: bool, + /// Buffer offset range, to look for words. + pub range: Range, +} + fn indent_size_for_line(text: &text::BufferSnapshot, row: u32) -> IndentSize { indent_size_for_text(text.chars_at(Point::new(row, 0))) } diff --git a/crates/language/src/buffer_tests.rs b/crates/language/src/buffer_tests.rs index 78f10640c9..70da611b2b 100644 --- a/crates/language/src/buffer_tests.rs +++ b/crates/language/src/buffer_tests.rs @@ -3145,7 +3145,11 @@ fn test_trailing_whitespace_ranges(mut rng: StdRng) { fn test_words_in_range(cx: &mut gpui::App) { init_settings(cx, |_| {}); - let contents = r#"let word=öäpple.bar你 Öäpple word2-öÄpPlE-Pizza-word ÖÄPPLE word"#; + // The first line are words excluded from the results with heuristics, we do not expect them in the test assertions. + let contents = r#" +0_isize 123 3.4 4   +let word=öäpple.bar你 Öäpple word2-öÄpPlE-Pizza-word ÖÄPPLE word + "#; let buffer = cx.new(|cx| { let buffer = Buffer::local(contents, cx).with_language(Arc::new(rust_lang()), cx); @@ -3159,7 +3163,11 @@ fn test_words_in_range(cx: &mut gpui::App) { assert_eq!( BTreeSet::from_iter(["Pizza".to_string()]), snapshot - .words_in_range(Some("piz"), 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: Some("piz"), + skip_digits: true, + range: 0..snapshot.len(), + }) .into_keys() .collect::>() ); @@ -3171,7 +3179,11 @@ fn test_words_in_range(cx: &mut gpui::App) { "ÖÄPPLE".to_string(), ]), snapshot - .words_in_range(Some("öp"), 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: Some("öp"), + skip_digits: true, + range: 0..snapshot.len(), + }) .into_keys() .collect::>() ); @@ -3183,28 +3195,44 @@ fn test_words_in_range(cx: &mut gpui::App) { "öäpple".to_string(), ]), snapshot - .words_in_range(Some("öÄ"), 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: Some("öÄ"), + skip_digits: true, + range: 0..snapshot.len(), + }) .into_keys() .collect::>() ); assert_eq!( BTreeSet::default(), snapshot - .words_in_range(Some("öÄ好"), 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: Some("öÄ好"), + skip_digits: true, + range: 0..snapshot.len(), + }) .into_keys() .collect::>() ); assert_eq!( BTreeSet::from_iter(["bar你".to_string(),]), snapshot - .words_in_range(Some("你"), 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: Some("你"), + skip_digits: true, + range: 0..snapshot.len(), + }) .into_keys() .collect::>() ); assert_eq!( BTreeSet::default(), snapshot - .words_in_range(Some(""), 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: Some(""), + skip_digits: true, + range: 0..snapshot.len(), + },) .into_keys() .collect::>() ); @@ -3221,7 +3249,36 @@ fn test_words_in_range(cx: &mut gpui::App) { "word2".to_string(), ]), snapshot - .words_in_range(None, 0..snapshot.len()) + .words_in_range(WordsQuery { + fuzzy_contents: None, + skip_digits: true, + range: 0..snapshot.len(), + }) + .into_keys() + .collect::>() + ); + assert_eq!( + BTreeSet::from_iter([ + "0_isize".to_string(), + "123".to_string(), + "3".to_string(), + "4".to_string(), + "bar你".to_string(), + "öÄpPlE".to_string(), + "Öäpple".to_string(), + "ÖÄPPLE".to_string(), + "öäpple".to_string(), + "let".to_string(), + "Pizza".to_string(), + "word".to_string(), + "word2".to_string(), + ]), + snapshot + .words_in_range(WordsQuery { + fuzzy_contents: None, + skip_digits: false, + range: 0..snapshot.len(), + }) .into_keys() .collect::>() ); diff --git a/crates/language/src/language_settings.rs b/crates/language/src/language_settings.rs index 5f51c5c2ee..dcfc55c8ff 100644 --- a/crates/language/src/language_settings.rs +++ b/crates/language/src/language_settings.rs @@ -326,8 +326,8 @@ pub struct CompletionSettings { /// When fetching LSP completions, determines how long to wait for a response of a particular server. /// When set to 0, waits indefinitely. /// - /// Default: 500 - #[serde(default = "lsp_fetch_timeout_ms")] + /// Default: 0 + #[serde(default = "default_lsp_fetch_timeout_ms")] pub lsp_fetch_timeout_ms: u64, } @@ -335,12 +335,13 @@ pub struct CompletionSettings { #[derive(Copy, Clone, Debug, Serialize, Deserialize, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum WordsCompletionMode { - /// Always fetch document's words for completions. + /// Always fetch document's words for completions along with LSP completions. Enabled, - /// Only if LSP response errors/times out/is empty, + /// Only if LSP response errors or times out, /// use document's words to show completions. Fallback, /// Never fetch or complete document's words for completions. + /// (Word-based completions can still be queried via a separate action) Disabled, } @@ -348,8 +349,8 @@ fn default_words_completion_mode() -> WordsCompletionMode { WordsCompletionMode::Fallback } -fn lsp_fetch_timeout_ms() -> u64 { - 500 +fn default_lsp_fetch_timeout_ms() -> u64 { + 0 } /// The settings for a particular language. diff --git a/crates/multi_buffer/src/multi_buffer.rs b/crates/multi_buffer/src/multi_buffer.rs index a6158a57eb..9cb2fba6f1 100644 --- a/crates/multi_buffer/src/multi_buffer.rs +++ b/crates/multi_buffer/src/multi_buffer.rs @@ -7299,14 +7299,6 @@ impl Iterator for MultiBufferRows<'_> { let overshoot = self.point - region.range.start; let buffer_point = region.buffer_range.start + overshoot; - // dbg!( - // buffer_point.row, - // region.range.end.column, - // self.point.row, - // region.range.end.row, - // self.cursor.is_at_end_of_excerpt(), - // region.buffer.max_point().row - // ); let expand_info = if self.is_singleton { None } else { diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 3f15d0d25a..81afa08813 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -4402,7 +4402,7 @@ impl LspStore { position: PointUtf16, context: CompletionContext, cx: &mut Context, - ) -> Task>> { + ) -> Task>>> { let language_registry = self.languages.clone(); if let Some((upstream_client, project_id)) = self.upstream_client() { @@ -4430,7 +4430,7 @@ impl LspStore { let mut result = Vec::new(); populate_labels_for_completions(completions, language, lsp_adapter, &mut result) .await; - Ok(result) + Ok(Some(result)) }) } else if let Some(local) = self.as_local() { let snapshot = buffer.read(cx).snapshot(); @@ -4444,7 +4444,7 @@ impl LspStore { ) .completions; if !completion_settings.lsp { - return Task::ready(Ok(Vec::new())); + return Task::ready(Ok(None)); } let server_ids: Vec<_> = buffer.update(cx, |buffer, cx| { @@ -4495,13 +4495,14 @@ impl LspStore { ).fuse(); let new_task = cx.background_spawn(async move { select_biased! { - response = lsp_request => response, + response = lsp_request => anyhow::Ok(Some(response?)), timeout_happened = timeout => { if timeout_happened { log::warn!("Fetching completions from server {server_id} timed out, timeout ms: {}", completion_settings.lsp_fetch_timeout_ms); - return anyhow::Ok(Vec::new()) + Ok(None) } else { - lsp_request.await + let completions = lsp_request.await?; + Ok(Some(completions)) } }, } @@ -4510,9 +4511,11 @@ impl LspStore { } })?; + let mut has_completions_returned = false; let mut completions = Vec::new(); for (lsp_adapter, task) in tasks { - if let Ok(new_completions) = task.await { + if let Ok(Some(new_completions)) = task.await { + has_completions_returned = true; populate_labels_for_completions( new_completions, language.clone(), @@ -4522,8 +4525,11 @@ impl LspStore { .await; } } - - Ok(completions) + if has_completions_returned { + Ok(Some(completions)) + } else { + Ok(None) + } }) } else { Task::ready(Err(anyhow!("No upstream client or local language server"))) diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index 038cc544f8..6593b7875d 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -3142,7 +3142,7 @@ impl Project { position: T, context: CompletionContext, cx: &mut Context, - ) -> Task>> { + ) -> Task>>> { let position = position.to_point_utf16(buffer.read(cx)); self.lsp_store.update(cx, |lsp_store, cx| { lsp_store.completions(buffer, position, context, cx) diff --git a/crates/project/src/project_tests.rs b/crates/project/src/project_tests.rs index 45ae65132b..fe2a49300a 100644 --- a/crates/project/src/project_tests.rs +++ b/crates/project/src/project_tests.rs @@ -2825,7 +2825,7 @@ async fn test_completions_without_edit_ranges(cx: &mut gpui::TestAppContext) { }) .next() .await; - let completions = completions.await.unwrap(); + let completions = completions.await.unwrap().unwrap(); let snapshot = buffer.update(cx, |buffer, _| buffer.snapshot()); assert_eq!(completions.len(), 1); assert_eq!(completions[0].new_text, "fullyQualifiedName"); @@ -2851,7 +2851,7 @@ async fn test_completions_without_edit_ranges(cx: &mut gpui::TestAppContext) { }) .next() .await; - let completions = completions.await.unwrap(); + let completions = completions.await.unwrap().unwrap(); let snapshot = buffer.update(cx, |buffer, _| buffer.snapshot()); assert_eq!(completions.len(), 1); assert_eq!(completions[0].new_text, "component"); @@ -2919,7 +2919,7 @@ async fn test_completions_with_carriage_returns(cx: &mut gpui::TestAppContext) { }) .next() .await; - let completions = completions.await.unwrap(); + let completions = completions.await.unwrap().unwrap(); assert_eq!(completions.len(), 1); assert_eq!(completions[0].new_text, "fully\nQualified\nName"); } diff --git a/crates/remote_server/src/remote_editing_tests.rs b/crates/remote_server/src/remote_editing_tests.rs index d0e1722e6a..35e87fbe93 100644 --- a/crates/remote_server/src/remote_editing_tests.rs +++ b/crates/remote_server/src/remote_editing_tests.rs @@ -506,7 +506,11 @@ async fn test_remote_lsp(cx: &mut TestAppContext, server_cx: &mut TestAppContext .unwrap(); assert_eq!( - result.into_iter().map(|c| c.label.text).collect::>(), + result + .unwrap() + .into_iter() + .map(|c| c.label.text) + .collect::>(), vec!["boop".to_string()] );