Skip to content

Commit

Permalink
Use more LSP data when falling back to regular completions label (zed…
Browse files Browse the repository at this point in the history
…-industries#23909)

Closes zed-industries#23590
Closes https://x.com/steeve/status/1865129235536568555

Before:

<img width="773" alt="before"
src="https://github.com/user-attachments/assets/129a8d12-9298-4bf5-8f2d-b3292c2562bf"
/>


After:

<img width="768" alt="after"
src="https://github.com/user-attachments/assets/e0516fb3-b02a-48be-8923-63bba05fdb69"
/>


The list obviously needs some solution for the cut-off part of the
completion label, but this is the reality for all extensions'
completions too, so one step at a time.


Release Notes:

- Improved default completion label fallback
  • Loading branch information
SomeoneToIgnore authored Jan 30, 2025
1 parent 48dba9a commit 9e45557
Show file tree
Hide file tree
Showing 3 changed files with 62 additions and 12 deletions.
6 changes: 3 additions & 3 deletions crates/editor/src/editor_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11282,7 +11282,7 @@ async fn test_completions_resolve_updates_labels_if_filter_text_matches(
cx.simulate_keystroke(".");

let item1 = lsp::CompletionItem {
label: "id".to_string(),
label: "method id()".to_string(),
filter_text: Some("id".to_string()),
detail: None,
documentation: None,
Expand Down Expand Up @@ -11332,7 +11332,7 @@ async fn test_completions_resolve_updates_labels_if_filter_text_matches(
.iter()
.map(|completion| &completion.label.text)
.collect::<Vec<_>>(),
vec!["id", "other"]
vec!["method id()", "other"]
)
}
CodeContextMenu::CodeActions(_) => panic!("Should show the completions menu"),
Expand Down Expand Up @@ -11387,7 +11387,7 @@ async fn test_completions_resolve_updates_labels_if_filter_text_matches(
.iter()
.map(|completion| &completion.label.text)
.collect::<Vec<_>>(),
vec!["method id()", "other"],
vec!["method id() Now resolved!", "other"],
"Should update first completion label, but not second as the filter text did not match."
);
}
Expand Down
52 changes: 52 additions & 0 deletions crates/language/src/language.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1750,6 +1750,58 @@ impl Grammar {
}

impl CodeLabel {
pub fn fallback_for_completion(
item: &lsp::CompletionItem,
language: Option<&Language>,
) -> Self {
let highlight_id = item.kind.and_then(|kind| {
let grammar = language?.grammar()?;
use lsp::CompletionItemKind as Kind;
match kind {
Kind::CLASS => grammar.highlight_id_for_name("type"),
Kind::CONSTANT => grammar.highlight_id_for_name("constant"),
Kind::CONSTRUCTOR => grammar.highlight_id_for_name("constructor"),
Kind::ENUM => grammar
.highlight_id_for_name("enum")
.or_else(|| grammar.highlight_id_for_name("type")),
Kind::FIELD => grammar.highlight_id_for_name("property"),
Kind::FUNCTION => grammar.highlight_id_for_name("function"),
Kind::INTERFACE => grammar.highlight_id_for_name("type"),
Kind::METHOD => grammar
.highlight_id_for_name("function.method")
.or_else(|| grammar.highlight_id_for_name("function")),
Kind::OPERATOR => grammar.highlight_id_for_name("operator"),
Kind::PROPERTY => grammar.highlight_id_for_name("property"),
Kind::STRUCT => grammar.highlight_id_for_name("type"),
Kind::VARIABLE => grammar.highlight_id_for_name("variable"),
Kind::KEYWORD => grammar.highlight_id_for_name("keyword"),
_ => None,
}
});

let label = &item.label;
let label_length = label.len();
let runs = highlight_id
.map(|highlight_id| vec![(0..label_length, highlight_id)])
.unwrap_or_default();
let text = if let Some(detail) = &item.detail {
format!("{label} {detail}")
} else if let Some(description) = item
.label_details
.as_ref()
.and_then(|label_details| label_details.description.as_ref())
{
format!("{label} {description}")
} else {
label.clone()
};
Self {
text,
runs,
filter_range: 0..label_length,
}
}

pub fn plain(text: String, filter_text: Option<&str>) -> Self {
let mut result = Self {
runs: Vec::new(),
Expand Down
16 changes: 7 additions & 9 deletions crates/project/src/lsp_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4380,7 +4380,8 @@ impl LspStore {

// NB: Zed does not have `details` inside the completion resolve capabilities, but certain language servers violate the spec and do not return `details` immediately, e.g. https://github.com/yioneko/vtsls/issues/213
// So we have to update the label here anyway...
let mut new_label = match snapshot.language() {
let language = snapshot.language();
let mut new_label = match language {
Some(language) => {
adapter
.labels_for_completions(&[completion_item.clone()], language)
Expand All @@ -4391,9 +4392,9 @@ impl LspStore {
.pop()
.flatten()
.unwrap_or_else(|| {
CodeLabel::plain(
completion_item.label,
completion_item.filter_text.as_deref(),
CodeLabel::fallback_for_completion(
&completion_item,
language.map(|language| language.as_ref()),
)
});
ensure_uniform_list_compatible_label(&mut new_label);
Expand Down Expand Up @@ -8079,10 +8080,7 @@ async fn populate_labels_for_completions(
};

let mut label = label.unwrap_or_else(|| {
CodeLabel::plain(
lsp_completion.label.clone(),
lsp_completion.filter_text.as_deref(),
)
CodeLabel::fallback_for_completion(&lsp_completion, language.as_deref())
});
ensure_uniform_list_compatible_label(&mut label);

Expand Down Expand Up @@ -8883,7 +8881,7 @@ fn include_text(server: &lsp::LanguageServer) -> Option<bool> {
/// Completion items are displayed in a `UniformList`.
/// Usually, those items are single-line strings, but in LSP responses,
/// completion items `label`, `detail` and `label_details.description` may contain newlines or long spaces.
/// Many language plugins construct these items by joining these parts together, and we may fall back to `CodeLabel::plain` that uses `label`.
/// Many language plugins construct these items by joining these parts together, and we may use `CodeLabel::fallback_for_completion` that uses `label` at least.
/// All that may lead to a newline being inserted into resulting `CodeLabel.text`, which will force `UniformList` to bloat each entry to occupy more space,
/// breaking the completions menu presentation.
///
Expand Down

0 comments on commit 9e45557

Please sign in to comment.