-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Conversation
There was a problem hiding this 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()],
}),
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) |
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) |
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 |
you should still try to resolve the command tough in case its none |
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