mirror of
https://github.com/zed-industries/zed.git
synced 2026-08-14 19:44:27 +00:00
Respect the filterText of LSP completion items during completion filtering (#62433)
# Objective
Closes #61646.
For code completions, Zed currently fuzzy-matches against
`CodeLabel::filter_text()`:
d4010e91cc/crates/editor/src/code_context_menus.rs (L337-L343)
`CodeLabel::filter_text()` is a substring of `CodeLabel.text`, which is
essentially the text itself. `CodeLabel.text` is constructed by Zed's
per-language adapters from the `label` and `detail` fields of the
completion items returned by the language server — the exact
construction differs from adapter to adapter, but the source data is the
same. In effect, `CodeLabel.text` ≈ `label` + `detail`. Zed therefore
filters on the server-returned `label` and `detail`, while the
server-returned `filterText` field is silently ignored.Per the LSP spec:
```
/**
* A string that should be used when filtering a set of
* completion items. When omitted, the label is used as the
* filter text for this item.
*/
filterText?: string;
```
we should use `filterText` when it is provided.
Normally, language servers populate `filterText` as a substring of
`label`, so the current behavior works fine. But for certain language
servers or functions, `filterText` can be entirely unrelated to `label`
and `detail`. For example, for `std::path::Path::parent()` in Rust,
rust-analyzer returns:
```json
{
"label": "parent()",
"labelDetails": {
"detail": "(alias dirname)",
"description": "fn(&self) -> Option<&Path>"
},
"kind": 2,
"preselect": true,
"sortText": "7ffffff6",
"filterText": "parentdirname",
...
}
```
Typing `dirname` therefore never surfaces this completion.
The root design issue behind this bug is that `CodeLabel` is not
well-suited to filtering LSP completions.
## Solution
`CodeLabel` and its related methods are kept untouched: the struct is
reused across the repo and is only unsuitable for filtering LSP
completions. Instead, the changes are made in `CompletionSource` and
`Completion`, each gaining a `filter_text()` method:
- `CompletionSource::filter_text()` handles LSP completions, returning
the server-provided `filterText` and falling back to the `label` when
`filterText` is absent.
- `Completion::filter_text()` is the general entry point used for fuzzy
matching; for non-LSP completions it falls back to the existing
`label.filter_text()`.
The fuzzy match target is switched from `CodeLabel::filter_text()` to
`Completion::filter_text()` — that is the core change.
Since the fuzzy match target is no longer guaranteed to be a substring
of the displayed `CodeLabel.text`, the matched characters no longer have
a direct position in the displayed text to highlight. Bold highlights
are therefore only rendered when `CodeLabel::filter_text()` equals
`Completion::filter_text()`. This is a safe choice, though not an ideal
one.
## Testing
Added a new GPUI test covering the new behavior; also built and tested
with a before/after comparison, attached in the Showcase section.
## Self-Review Checklist:
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
## Showcase
| Before | After |
|:--:|:--:|
| <img width="708" height="308" alt="Before"
src="https://github.com/user-attachments/assets/ca2e3820-7ea1-4dcc-a91f-28aab71aecc5"
/> | <img width="696" height="248" alt="After"
src="https://github.com/user-attachments/assets/334e240b-64d5-495b-aef6-772456b993ba"
/> |
---
Release Notes:
- Improved completion filtering for lsp completions.
This commit is contained in:
parent
a034d87024
commit
52894d3f48
6 changed files with 216 additions and 46 deletions
|
|
@ -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();
|
||||
|
|
|
|||
|
|
@ -254,6 +254,7 @@ pub struct CompletionsMenu {
|
|||
pub completions: Rc<RefCell<Box<[Completion]>>>,
|
||||
/// String match candidate for each completion, grouped by `match_start`.
|
||||
match_candidates: Arc<[(Option<text::Anchor>, Vec<StringMatchCandidate>)]>,
|
||||
label_match_state: LabelMatchState,
|
||||
/// Entries displayed in the menu, which is a filtered and sorted subset of `match_candidates`.
|
||||
pub entries: Rc<RefCell<Box<[CompletionMenuEntry]>>>,
|
||||
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<StringMatch>,
|
||||
/// Fuzzy match result against the displayed label
|
||||
label_matches: Box<[Option<StringMatch>]>,
|
||||
}
|
||||
|
||||
/// 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<RefCell<Box<[Option<StringMatch>]>>>,
|
||||
}
|
||||
|
||||
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<UniformListScrollHandle>,
|
||||
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<Buffer>,
|
||||
cx: &Context<Editor>,
|
||||
) -> Task<Vec<StringMatch>> {
|
||||
) -> Task<CompletionMatchResults> {
|
||||
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::<Box<[_]>>();
|
||||
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<StringMatch>,
|
||||
match_results: CompletionMatchResults,
|
||||
provider: Option<Rc<dyn CompletionProvider>>,
|
||||
window: &mut Window,
|
||||
cx: &mut Context<Editor>,
|
||||
) {
|
||||
let CompletionMatchResults {
|
||||
filter_matches,
|
||||
label_matches,
|
||||
} = match_results;
|
||||
let completions = self.completions.borrow();
|
||||
let mut entries: Vec<CompletionMenuEntry> = Vec::with_capacity(matches.len());
|
||||
let mut entries: Vec<CompletionMenuEntry> = 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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
}),
|
||||
|
|
|
|||
|
|
@ -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::<lsp::request::Completion, _, _>(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, |_| {});
|
||||
|
|
|
|||
|
|
@ -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(())
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Cow<'_, lsp::CompletionItem>> {
|
||||
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) {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue