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

LSP: update OmniSharpCodeHandler so we support codeAction/resolve #2147

Closed

Conversation

razzmatazz
Copy link
Contributor

This feature was introduced in LSP
3.16.0 https://microsoft.github.io/language-server-protocol/specification#codeAction_resolve
and allows us to drop the hack where we were introducing a special
"omnisharp/executeCodeAction" command to delay the resolution of code
action changesets until user actually selets that code
action: see #1814.

Related to:

@razzmatazz
Copy link
Contributor Author

razzmatazz commented May 2, 2021

@david-driscoll I need some guidance how to figure out if client supports codeAction/resolve as I want to add this code to OmniSharpCodeActionHandler.cs so we support clients that don't have support for codeAction/resolve

                if (_server.ClientSettings.Capabilities.TextDocument.CodeActionResolve)
                {
                    // new behaviour with CodeAction+Data delay-resolved via resolve handler
                }
                else
                {
                    // old behaviour with RunCodeActionResponse() where we 
                    // precompute changeset that was removed with #1814
                }

the PR is marked as a draft as I cannot discover .CodeActionResolve on clientsettings.capabilities.textdoc.. what is the way to figure out what if there was a support negotiated support between server and the client for this feature in handler code ?

@razzmatazz razzmatazz force-pushed the code-action-resolve-support branch 3 times, most recently from 119157b to dcea55f Compare May 8, 2021 13:14
@razzmatazz
Copy link
Contributor Author

@david-driscoll I need some guidance how to figure out if client supports codeAction/resolve

Ok, I figured it out

@razzmatazz razzmatazz marked this pull request as ready for review May 8, 2021 14:12
@razzmatazz
Copy link
Contributor Author

@filipw this ready for review

functionally nothing much changes but we now will use std functionality from LSP to resolve-on-demand code action edits

for older lsp clients we will fallback to eager edit resolving (which can be slow on large projects)

@razzmatazz
Copy link
Contributor Author

@filipw this ready for review

functionally nothing much changes but we now will use std functionality from LSP to resolve-on-demand code action edits

for older lsp clients we will fallback to eager edit resolving (which can be slow on large projects)

oops, some of the tests are still failing..

This feature was introduced in LSP
3.16.0 https://microsoft.github.io/language-server-protocol/specification#codeAction_resolve
and allows us to drop the hack where we were introducing a special
"omnisharp/executeCodeAction" command to delay the resolution of code
action changesets until user actually selets that code
action: see OmniSharp#1814.

Related to:
- OmniSharp#2068
- OmniSharp#1814
@razzmatazz razzmatazz force-pushed the code-action-resolve-support branch from 32a596c to 102a0de Compare May 12, 2021 20:31
@razzmatazz
Copy link
Contributor Author

@david-driscoll maybe you have some feedback?

Copy link
Contributor

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

@david-driscoll could you take a look?

Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

LGTM thanks

@JoeRobich
Copy link
Member

superseded by #2467

@JoeRobich JoeRobich closed this Sep 14, 2023
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