diff --git a/crates/editor/src/code_completion_tests.rs b/crates/editor/src/code_completion_tests.rs index babcbede3bc..27062801efb 100644 --- a/crates/editor/src/code_completion_tests.rs +++ b/crates/editor/src/code_completion_tests.rs @@ -501,7 +501,7 @@ async fn filter_and_sort_matches( let candidates: Arc<[StringMatchCandidate]> = completions .iter() .enumerate() - .map(|(id, completion)| StringMatchCandidate::new(id, completion.label.filter_text())) + .map(|(id, completion)| StringMatchCandidate::new(id, completion.filter_text())) .collect(); let cancel_flag = Arc::new(AtomicBool::new(false)); let background_executor = cx.executor(); diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index a45236208e4..209fdf13bd7 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -254,6 +254,7 @@ pub struct CompletionsMenu { pub completions: Rc>>, /// String match candidate for each completion, grouped by `match_start`. match_candidates: Arc<[(Option, Vec)]>, + label_match_state: LabelMatchState, /// Entries displayed in the menu, which is a filtered and sorted subset of `match_candidates`. pub entries: Rc>>, pub selected_item: usize, @@ -300,6 +301,37 @@ pub enum CompletionsMenuSource { Words { ignore_threshold: bool }, } +pub struct CompletionMatchResults { + /// Fuzzy match result against the filter text + filter_matches: Vec, + /// Fuzzy match result against the displayed label + label_matches: Box<[Option]>, +} + +/// The state for a second fuzzy match against the displayed label, used to +/// render match highlights when the filter text differs from the label. +struct LabelMatchState { + /// Candidates for the label match + candidates: Arc<[StringMatchCandidate]>, + /// Results of the label match + label_matches: Rc]>>>, +} + +impl LabelMatchState { + fn new(completions: &[Completion]) -> Self { + let candidates = completions + .iter() + .enumerate() + .map(|(id, completion)| StringMatchCandidate::new(id, completion.label.filter_text())) + .collect(); + let matches = std::iter::repeat_n(None, completions.len()).collect(); + Self { + candidates, + label_matches: Rc::new(RefCell::new(matches)), + } + } +} + // TODO: There should really be a wrapper around fuzzy match tasks that does this. impl Drop for CompletionsMenu { fn drop(&mut self) { @@ -337,10 +369,11 @@ impl CompletionsMenu { let match_candidates = completions .iter() .enumerate() - .map(|(id, completion)| StringMatchCandidate::new(id, completion.label.filter_text())) + .map(|(id, completion)| StringMatchCandidate::new(id, completion.filter_text())) .into_group_map_by(|candidate| completions[candidate.id].match_start) .into_iter() .collect(); + let label_match_state = LabelMatchState::new(&completions); let completions_menu = Self { id, @@ -353,6 +386,7 @@ impl CompletionsMenu { show_completion_documentation, completions: RefCell::new(completions).into(), match_candidates, + label_match_state, entries: Rc::new(RefCell::new(Box::new([]))), selected_item: 0, filter_task: Task::ready(()), @@ -383,7 +417,7 @@ impl CompletionsMenu { scroll_handle: Option, snippet_sort_order: SnippetSortOrder, ) -> Self { - let completions = choices + let completions: Box<[Completion]> = choices .iter() .map(|choice| Completion { replace_range: selection.clone(), @@ -409,6 +443,7 @@ impl CompletionsMenu { .map(|(id, completion)| StringMatchCandidate::new(id, completion)) .collect(), )]); + let label_match_state = LabelMatchState::new(&completions); let entries = choices .iter() .enumerate() @@ -431,6 +466,7 @@ impl CompletionsMenu { buffer, completions: RefCell::new(completions).into(), match_candidates, + label_match_state, entries: RefCell::new(entries).into(), selected_item: 0, filter_task: Task::ready(()), @@ -924,6 +960,7 @@ impl CompletionsMenu { let selected_item = self.selected_item; let completions = self.completions.clone(); let entries = self.entries.clone(); + let label_matches = self.label_match_state.label_matches.clone(); let last_rendered_range = self.last_rendered_range.clone(); let style = style.clone(); let list = uniform_list( @@ -933,6 +970,7 @@ impl CompletionsMenu { last_rendered_range.borrow_mut().replace(range.clone()); let start_ix = range.start; let completions_guard = completions.borrow_mut(); + let label_matches_guard = label_matches.borrow(); entries.borrow()[range] .iter() @@ -963,13 +1001,29 @@ impl CompletionsMenu { let filter_start = completion.label.filter_range.start; + let match_highlights = label_matches_guard + .get(mat.candidate_id) + .and_then(Option::as_ref) + .filter(|label_match| { + // in case label changes during completion/resolve + label_match.string == completion.label.filter_text() + }) + .or_else(|| { + (completion.filter_text() == completion.label.filter_text()) + .then_some(mat) + }) + .into_iter() + .flat_map(|string_match| { + string_match.ranges().map(|range| { + ( + filter_start + range.start..filter_start + range.end, + FontWeight::BOLD.into(), + ) + }) + }); + let highlights = gpui::combine_highlights( - mat.ranges().map(|range| { - ( - filter_start + range.start..filter_start + range.end, - FontWeight::BOLD.into(), - ) - }), + match_highlights, styled_runs_for_code_label( &completion.label, &style.syntax, @@ -1308,10 +1362,11 @@ impl CompletionsMenu { query_end: text::Anchor, buffer: &Entity, cx: &Context, - ) -> Task> { + ) -> Task { let buffer_snapshot = buffer.read(cx).snapshot(); let background_executor = cx.background_executor().clone(); let match_candidates = self.match_candidates.clone(); + let label_match_candidates = self.label_match_state.candidates.clone(); let cancel_filter = self.cancel_filter.clone(); let default_query = query.clone(); @@ -1329,35 +1384,76 @@ impl CompletionsMenu { }) .collect_vec(); - let mut results = vec![]; + let mut filter_match_results = vec![]; + let mut label_match_results = + std::iter::repeat_n(None, label_match_candidates.len()).collect::>(); for (query, match_candidates) in queries_and_candidates { - results.extend( - fuzzy::match_strings( - &match_candidates, - &query, - query.chars().any(|c| c.is_uppercase()), - false, - 1000, - &cancel_filter, - background_executor.clone(), - ) - .await, - ); + let smart_case = query.chars().any(|character| character.is_uppercase()); + let filter_matches = fuzzy::match_strings( + &match_candidates, + &query, + smart_case, + false, + 1000, + &cancel_filter, + background_executor.clone(), + ) + .await; + if query.is_empty() { + filter_match_results.extend(filter_matches); + continue; + } + + // The filter text may differ from the displayed label, so the + // main match positions don't map onto the label; match the + // label separately to get positions for the highlights. When + // the filter text is the label itself, the main match already + // covers the label, so no second pass is needed. + let matching_label_candidates = filter_matches + .iter() + .filter_map(|filter_match| { + let label_candidate = + label_match_candidates.get(filter_match.candidate_id)?; + (filter_match.string != label_candidate.string).then_some(label_candidate) + }) + .collect_vec(); + if matching_label_candidates.is_empty() { + filter_match_results.extend(filter_matches); + continue; + } + let label_matches = fuzzy::match_strings( + &matching_label_candidates, + &query, + smart_case, + false, + matching_label_candidates.len(), + &cancel_filter, + background_executor.clone(), + ) + .await; + for label_match in label_matches { + let candidate_id = label_match.candidate_id; + label_match_results[candidate_id] = Some(label_match); + } + filter_match_results.extend(filter_matches); + } + CompletionMatchResults { + filter_matches: filter_match_results, + label_matches: label_match_results, } - results }); let completions = self.completions.clone(); let sort_completions = self.sort_completions; let snippet_sort_order = self.snippet_sort_order; cx.foreground_executor().spawn(async move { - let mut matches = matches_task.await; + let mut results = matches_task.await; let completions_ref = completions.borrow(); if sort_completions { - matches = Self::sort_string_matches( - matches, + results.filter_matches = Self::sort_string_matches( + results.filter_matches, Some(&query), // used for non-snippets only snippet_sort_order, &completions_ref, @@ -1367,28 +1463,32 @@ impl CompletionsMenu { // Remove duplicate snippet prefixes (e.g., "cool code" will match // the text "c c" in two places; we should only show the longer one) let mut snippets_seen = HashSet::<(usize, usize)>::default(); - matches.retain(|result| { + results.filter_matches.retain(|result| { match completions_ref[result.candidate_id].snippet_deduplication_key { Some(key) => snippets_seen.insert(key), None => true, } }); - matches + results }) } pub fn set_filter_results( &mut self, - matches: Vec, + match_results: CompletionMatchResults, provider: Option>, window: &mut Window, cx: &mut Context, ) { + let CompletionMatchResults { + filter_matches, + label_matches, + } = match_results; let completions = self.completions.borrow(); - let mut entries: Vec = Vec::with_capacity(matches.len()); + let mut entries: Vec = Vec::with_capacity(filter_matches.len()); let mut last_group: Option<&CompletionGroup> = None; - for mat in matches { + for mat in filter_matches { let group = completions[mat.candidate_id].group.as_ref(); if group != last_group { if group.is_some() || last_group.is_some() { @@ -1404,6 +1504,7 @@ impl CompletionsMenu { entries.push(CompletionMenuEntry::Match(mat)); } drop(completions); + *self.label_match_state.label_matches.borrow_mut() = label_matches; *self.entries.borrow_mut() = entries.into_boxed_slice(); self.selected_item = self.find_selectable_entry(0, true).unwrap_or(0); self.handle_selection_changed(provider.as_deref(), window, cx); @@ -1485,7 +1586,7 @@ impl CompletionsMenu { string_match, )); // This exact matching won't work for multi-word snippets, but it's fine - let sort_exact = Reverse(if Some(completion.label.filter_text()) == query { + let sort_exact = Reverse(if Some(completion.filter_text()) == query { 1 } else { 0 diff --git a/crates/editor/src/completions.rs b/crates/editor/src/completions.rs index e649c6dd623..10756afbbbf 100644 --- a/crates/editor/src/completions.rs +++ b/crates/editor/src/completions.rs @@ -1347,7 +1347,6 @@ fn snippet_completions( replace: lsp_range, }, )), - filter_text: Some(snippet.body.clone()), sort_text: Some(char::MAX.to_string()), ..lsp::CompletionItem::default() }), diff --git a/crates/editor/src/editor_tests.rs b/crates/editor/src/editor_tests.rs index 0a9400c568e..59247014568 100644 --- a/crates/editor/src/editor_tests.rs +++ b/crates/editor/src/editor_tests.rs @@ -20308,6 +20308,68 @@ async fn test_completion_in_multibuffer_with_newest_selection_in_other_buffer( }); } +#[gpui::test] +async fn test_completion_filter_text_need_not_be_label_substring(cx: &mut TestAppContext) { + init_test(cx, |language_settings| { + language_settings.defaults.completions = Some(CompletionSettingsContent { + words: Some(WordsCompletionMode::Disabled), + ..Default::default() + }); + }); + + let mut cx = EditorLspTestContext::new_rust( + lsp::ServerCapabilities { + completion_provider: Some(lsp::CompletionOptions::default()), + ..Default::default() + }, + cx, + ) + .await; + + cx.lsp + .set_request_handler::(move |_, _| async move { + Ok(Some(lsp::CompletionResponse::Array(vec![ + lsp::CompletionItem { + label: "rendered value".to_string(), + filter_text: Some("typed_prefix".to_string()), + ..Default::default() + }, + lsp::CompletionItem { + label: "typed_prefix fallback".to_string(), + ..Default::default() + }, + ]))) + }); + + cx.set_state("fn main() { typed_prefixˇ }"); + cx.update_editor(|editor, window, cx| { + editor.show_completions(&ShowCompletions, window, cx); + }); + + cx.run_until_parked(); + cx.condition(|editor, _| editor.context_menu_visible()) + .await; + + cx.update_editor(|editor, _, _| { + let context_menu = editor.context_menu.borrow(); + let Some(CodeContextMenu::Completions(menu)) = context_menu.as_ref() else { + panic!("expected completion menu to be open"); + }; + + let completions = menu.completions.borrow(); + let entries = menu.entries.borrow(); + let completion = entries + .iter() + .filter_map(|entry| entry.as_match()) + .map(|string_match| &completions[string_match.candidate_id]) + .find(|completion| completion.label.text() == "rendered value") + .expect("completion should be matched using its non-substring filterText"); + + assert_eq!(completion.label.filter_text(), "rendered value"); + assert_eq!(completion.filter_text(), "typed_prefix"); + }); +} + #[gpui::test] async fn test_completion(cx: &mut TestAppContext) { init_test(cx, |_| {}); diff --git a/crates/project/src/lsp_store.rs b/crates/project/src/lsp_store.rs index 58a2c12a532..f61d297e722 100644 --- a/crates/project/src/lsp_store.rs +++ b/crates/project/src/lsp_store.rs @@ -7321,18 +7321,7 @@ impl LspStore { let mut completions = completions.borrow_mut(); let completion = &mut completions[completion_index]; - if completion.label.filter_text() == new_label.filter_text() { - completion.label = new_label; - } else { - log::error!( - "Resolved completion changed display label from {} to {}. \ - Refusing to apply this because it changes the fuzzy match text from {} to {}", - completion.label.text(), - new_label.text(), - completion.label.filter_text(), - new_label.filter_text() - ); - } + completion.label = new_label; Ok(()) } diff --git a/crates/project/src/project.rs b/crates/project/src/project.rs index cb6e88d2964..20cfe3c69c0 100644 --- a/crates/project/src/project.rs +++ b/crates/project/src/project.rs @@ -627,6 +627,19 @@ impl CompletionSource { } } + pub fn filter_text(&self) -> Option<&str> { + if let Self::Lsp { lsp_completion, .. } = self { + Some( + lsp_completion + .filter_text + .as_deref() + .unwrap_or(lsp_completion.label.as_str()), + ) + } else { + None + } + } + pub fn lsp_completion(&self, apply_defaults: bool) -> Option> { if let Self::Lsp { lsp_completion, @@ -6762,6 +6775,12 @@ impl Completion { .map(|lsp_completion| lsp_completion.label.clone()) } + pub fn filter_text(&self) -> &str { + self.source + .filter_text() + .unwrap_or_else(|| self.label.filter_text()) + } + /// A key that can be used to sort completions when displaying /// them to the user. pub fn sort_key(&self) -> (usize, &str) {