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

feat: resolve code action #7677

Merged
merged 6 commits into from
Jul 21, 2023
Merged

feat: resolve code action #7677

merged 6 commits into from
Jul 21, 2023

Conversation

sigmaSd
Copy link
Contributor

@sigmaSd sigmaSd commented Jul 19, 2023

fix #5118

tested with deno (works)

I imagine the code is not ideal since I'm blocking for a future inside the callback to wait for the resolver also I had to clone the code action

Copy link
Member

@pascalkuthe pascalkuthe left a comment

Choose a reason for hiding this comment

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

instead of blocking on the future you can/should add new job that applies the edit (and the command)

You also need to add support for resolving commands, and set the capabilities correctly. While you are it, it would be good to add some capabilities we are missing. RA tends to frreqendly ignore capabilities so that meant we forgot to add them sometimes. Simply add the following to the code action capabilities

                        is_preferred_support: Some(true),
                        disabled_support: Some(true),
                        data_support: Some(true),
                        resolve_support: Some(CodeActionCapabilityResolveSupport {
                            properties: vec!["edit".to_owned(), "command".to_owned()],
                        }),

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Jul 19, 2023

As far as I can tell, I can't nest jobs callback, I searched the code base and there is no other instance of nesting callbacks

There seems to be a similar lsp resolve call, but its registered to be triggered with idle timeout https://github.com/helix-editor/helix/blob/master/helix-term/src/ui/editor.rs#L1010 (which require saving state in the editor)

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Jul 19, 2023

Another similar thing is this https://github.com/helix-editor/helix/blob/master/helix-term/src/commands.rs#L4513

set_completion does block internally (it uses tokio_blockon internally in Completion::new to resolve items)

@pascalkuthe
Copy link
Member

hmm right I forgot this was already inside a box, I have a patch locally to allow that but its a bit more involved so that's probably ok for now

@pascalkuthe
Copy link
Member

you should still try to resolve the command tough in case its none

@pascalkuthe pascalkuthe added E-easy Call for participation: Experience needed to fix: Easy / not much A-language-server Area: Language server client S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 19, 2023
@pascalkuthe pascalkuthe added S-waiting-on-review Status: Awaiting review from a maintainer. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jul 19, 2023
@pascalkuthe pascalkuthe added this to the next milestone Jul 20, 2023
@the-mikedavis the-mikedavis merged commit 8977123 into helix-editor:master Jul 21, 2023
mtoohey31 pushed a commit to mtoohey31/helix that referenced this pull request Jun 2, 2024
smortime pushed a commit to smortime/helix that referenced this pull request Jul 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-language-server Area: Language server client E-easy Call for participation: Experience needed to fix: Easy / not much S-waiting-on-review Status: Awaiting review from a maintainer.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support lsp "codeAction/resolve"
3 participants