Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix completion details on new clangd versions #25405

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

BorisVassilev1
Copy link

@BorisVassilev1 BorisVassilev1 commented Feb 22, 2025

Fixes #16057

In newer versions of clangd, the switch labelDetailsSupport in the json passed to the language server modifies the format of the returned json. Zed handles well the old format, but misses the function parameters in the new one. For example:
The old format looks like this:

...
"label": " Window(int width, int height, const char *name, bool vsync, bool resizable)",
...

and with labelDetailsSupport = true:

...
 "label": " Window",
 "labelDetails": {
     "detail": "(int width, int height, const char *name, bool vsync, bool resizable)"
 },
...

A simple solution is to just to not tell the language server that label details are supported and force it to use the old format. This is a dirty fix, but makes the completions behave like in the old versions of clangd.

I do not know if this will break another language server. From what I've found out most lsp-s do not depend on that setting and provide all completion data either way. If not, this switch will need to be exposed in a config or be at least lsp-dependant.

Lastly, I do not know Rust, maybe will need help to make a better fix for the issue.

Release Notes:

  • Fixed broken C++ completion suggestions

Copy link

cla-bot bot commented Feb 22, 2025

We require contributors to sign our Contributor License Agreement, and we don't have @BorisVassilev1 on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@BorisVassilev1
Copy link
Author

@cla-bot check

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Feb 22, 2025
Copy link

cla-bot bot commented Feb 22, 2025

The cla-bot has been summoned, and re-checked this pull request!

Copy link
Member

@maxdeviant maxdeviant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not know if this will break another language server.

There are other language servers that do depend on labelDetails, so this will certainly break them.

We'll need to find an alternative solution. Since this is related to a change in clangd, then the fix should be made accordingly in the code that deals specifically with clangd.

Here is where we construct the completion menu labels for clangd:

async fn label_for_completion(
&self,
completion: &lsp::CompletionItem,
language: &Arc<Language>,
) -> Option<CodeLabel> {
let label = completion
.label
.strip_prefix('•')
.unwrap_or(&completion.label)
.trim();
match completion.kind {
Some(lsp::CompletionItemKind::FIELD) if completion.detail.is_some() => {
let detail = completion.detail.as_ref().unwrap();
let text = format!("{} {}", detail, label);
let source = Rope::from(format!("struct S {{ {} }}", text).as_str());
let runs = language.highlight_text(&source, 11..11 + text.len());
return Some(CodeLabel {
filter_range: detail.len() + 1..text.len(),
text,
runs,
});
}
Some(lsp::CompletionItemKind::CONSTANT | lsp::CompletionItemKind::VARIABLE)
if completion.detail.is_some() =>
{
let detail = completion.detail.as_ref().unwrap();
let text = format!("{} {}", detail, label);
let runs = language.highlight_text(&Rope::from(text.as_str()), 0..text.len());
return Some(CodeLabel {
filter_range: detail.len() + 1..text.len(),
text,
runs,
});
}
Some(lsp::CompletionItemKind::FUNCTION | lsp::CompletionItemKind::METHOD)
if completion.detail.is_some() =>
{
let detail = completion.detail.as_ref().unwrap();
let text = format!("{} {}", detail, label);
let runs = language.highlight_text(&Rope::from(text.as_str()), 0..text.len());
let filter_start = detail.len() + 1;
let filter_end =
if let Some(end) = text.rfind('(').filter(|end| *end > filter_start) {
end
} else {
text.len()
};
return Some(CodeLabel {
filter_range: filter_start..filter_end,
text,
runs,
});
}
Some(kind) => {
let highlight_name = match kind {
lsp::CompletionItemKind::STRUCT
| lsp::CompletionItemKind::INTERFACE
| lsp::CompletionItemKind::CLASS
| lsp::CompletionItemKind::ENUM => Some("type"),
lsp::CompletionItemKind::ENUM_MEMBER => Some("variant"),
lsp::CompletionItemKind::KEYWORD => Some("keyword"),
lsp::CompletionItemKind::VALUE | lsp::CompletionItemKind::CONSTANT => {
Some("constant")
}
_ => None,
};
if let Some(highlight_id) = language
.grammar()
.and_then(|g| g.highlight_id_for_name(highlight_name?))
{
let mut label = CodeLabel::plain(label.to_string(), None);
label.runs.push((
0..label.text.rfind('(').unwrap_or(label.text.len()),
highlight_id,
));
return Some(label);
}
}
_ => {}
}
Some(CodeLabel::plain(label.to_string(), None))
}

@BorisVassilev1
Copy link
Author

BorisVassilev1 commented Feb 23, 2025

I don't know if this is the 'rustacean' way of doing this, but 'works on my machine'.

I have not been able to make all tests pass on my machine (even with a freshly cloned repository). I have followed all instructions (except those regarding collaboration (local server and such). Will have to rely on workflows.

How do I trigger workflows again? -> Edit: they were queued already.

Previous commit broke other language servers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

truncated language server suggestions - missing parameters (clangd)
2 participants