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

[WIP] Improve completions for TypeScript #10787

Closed
wants to merge 17 commits into from

Conversation

babyccino
Copy link
Contributor

Related to #5287

This is a WIP but I just wanted to check if this is something Zed would be interested in before I put more time into it. Basically what I've done is changed the LSP functionality so labels as well as documentation is updated from the LSP completionItem/resolve response.

Before:
Screenshot 2024-04-19 at 7 19 07 PM

After:
Screenshot 2024-04-19 at 7 18 51 PM

@cla-bot cla-bot bot added the cla-signed The user has signed the Contributor License Agreement label Apr 19, 2024
@maxbrunsfeld
Copy link
Collaborator

The new labels look great. Could you explain a bit more about why the separate labels_for_resolved_completion method is needed?

@babyccino
Copy link
Contributor Author

The new labels look great. Could you explain a bit more about why the separate labels_for_resolved_completion method is needed?

It's not completely necessary. You could definitely determine in the label function which kind of completion you're dealing with, but for the TS LSP (and maybe for others) the format is completely different depending on whether they are regular completions or resolved completions. I figured it was easier to do it this way and maybe there are some other LSPs where it's not so easy to differentiate.

@babyccino babyccino marked this pull request as ready for review April 28, 2024 11:04
@zed-industries-bot
Copy link

zed-industries-bot commented May 3, 2024

Warnings
⚠️

This PR is missing release notes.

Please add a "Release Notes" section that describes the change:

Release Notes:

- (Added|Fixed|Improved) ... ([#<public_issue_number_if_exists>](https://github.com/zed-industries/zed/issues/<public_issue_number_if_exists>)).

If your change is not user-facing, you can use "N/A" for the entry:

Release Notes:

- N/A

Generated by 🚫 dangerJS against f402297

@mrnugget
Copy link
Member

mrnugget commented May 3, 2024

Without having looked too closely at the code: @babyccino it might be worth checking against main and what has changed there, since I also changed the how we do completion item resolution: #11157

@jackTabsCode
Copy link
Contributor

Hi @mikayla-maki, I'm curious what the status of this is? Is it ready to be merged?

@babyccino babyccino changed the title Improve completions for TypeScript [WIP] Improve completions for TypeScript Jun 21, 2024
@babyccino
Copy link
Contributor Author

Hi @mikayla-maki, I'm curious what the status of this is? Is it ready to be merged?

I got distracted by #12106 so I haven't worked on this in a while. It's still in a very rough state and definitely not ready to be merged. If Zed is still interested in this I can work on it after #12106 is merged

@ConradIrwin
Copy link
Member

@babyccino I'm going to close for now, but please re-open when you get time to work on it!

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.

7 participants