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] Add support for code action quick fixes #19

Closed
wants to merge 12 commits into from

Conversation

danielmartin
Copy link
Contributor

[Still WIP]

Screenshot (VSCode):

screen shot 2018-11-26 at 9 59 06 pm

This PR adds support for the textDocument/codeAction LSP method, so that Swift diagnostics that carry a quick fix can be applied from the LSP client with a keystroke.

@benlangmuir
Copy link
Contributor

Very cool! One of the concerns I've had about using the codeAction API for fixits has been that the editor doesn't have a great way to keep the fixits in sync with the live diagnostics. Have you run into any synchronization issues testing this with vscode, or is it okay in practice?

@danielmartin
Copy link
Contributor Author

I have yet to filter the available diagnostic fixits so that the VSCode light bulb only offers those that are relevant to where the cursor is, not every fixit for the current file, but it's looking good so far.

I think it would also make the interface with clangd simpler, as clangd is also using codeActions for quick fixes. Refactor actions also use this method, with the difference that, in the case of refactors, there's an extra step where the client sends the command to be executed to the server.

I had a previous implementation where the fixits simply came with the diagnostics in publishDiagnostics and that worked beautifully in Emacs but it didn't show a light bulb in VSCode. AFAIK that MS UI requires that the server answers to codeAction calls.

@benlangmuir
Copy link
Contributor

@danielmartin FWIW we are planning to add the fixits to publishDiagnostics as well - see https://reviews.llvm.org/D50415 for clangd's implementation of that.

@cgarciae
Copy link

I'd love to see this feature become a reality!
Would be really useful for implementing protocols, right now I have to guess the correct functions.

@benlangmuir
Copy link
Contributor

I believe this PR is obseleted by the combination of
#70
#216
#238

If there's something I missed, feel free to reopen after rebasing.

@benlangmuir benlangmuir closed this Sep 2, 2020
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.

3 participants