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

Add package information in completions #616

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

lionel-
Copy link
Contributor

@lionel- lionel- commented Oct 31, 2024

@DavisVaughan
Copy link
Contributor

DavisVaughan commented Oct 31, 2024

I think we also need to let the client know we support this
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_completion

Look for labelDetailsSupport there.

It's possible we could be working with a client that doesn't support this, but it is >3 years old now so its probably ok to ignore the client supplied option for now (we should still set the server side one though)

Comment on lines +179 to +181
let detail = format!("({})", parameters.joined(", "));
let label_details = item_details(Some(detail), package);
item.label_details = Some(label_details);
Copy link
Contributor

@DavisVaughan DavisVaughan Oct 31, 2024

Choose a reason for hiding this comment

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

I agree that we have many ways to see the function parameters, and by far the most useless one is seeing the function parameters in this completion item menu - the parameters are very scrunched together and you can't see their defaults and they are often cut off. Plus, the point of this menu is to pick the function, not supply the arguments, that comes later

I vote for just showing the package name, and doing a little work to strip out parameters here all together, since its probably a non trivial amount of work to compute it and we won't need it anymore.

This comment was marked as off-topic.

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants