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

Support the 'reference' codeActionKind #525

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nemethf
Copy link
Contributor

@nemethf nemethf commented Nov 18, 2019

I wasn't sure whether my idea of supporting extra reference methods with a slight extension of the language server protocol would work, so I implemented this proof-of-concept. Unfortunately, it turned out that I don't really know c++. On the other hand, the code works. The corresponding code implementing the client side is here: https://github.com/nemethf/eglot-x/tree/reference-codeActionKind

And this is the summary of the 'reference' codeActionKind extension:
joaotavora/eglot#302 (comment)

I hope you find this useful, and I'm eager to receive your feedback.

@MaskRay
Copy link
Owner

MaskRay commented Nov 29, 2019

Thanks for working on this. My feeling regarding joaotavora/eglot#302 (comment) is that to make less use of extension methods, it adds some out-of-place round trips. It does reduce the number of methods, but is it really worthwhile? There are still some extensions to CodeAction anyway.

@nemethf
Copy link
Contributor Author

nemethf commented Nov 30, 2019

My aim is to come up with a server-agnostic approach. Currently if clangd implements the same $ccls/* methods, then the clients need to be modified to add special cases for the clangd server as well.

It is true that the reference actionKind approach does not expose all the capabilities of the $ccls methods, but it provides a useful subset. Clients can support additional features with server-specific code. I think the extra round-trip does not result in noticeable extra delay, because the clients should wait for user input between the two method calls.

An alternative approach is to list the names of the extra reference methods among the server's capabilities. However, this approach does not support additional arguments. Would you accept this?

is it really worthwhile?

I looked at a bunch of LSP servers. It seems ccls is the only one that has this kind of protocol extensions. So, at the moment, it's more like a theoretical change than a practical one.

@MaskRay MaskRay force-pushed the master branch 2 times, most recently from b299069 to de800eb Compare April 22, 2020 15:44
@MaskRay MaskRay force-pushed the master branch 2 times, most recently from 7818055 to 99f0b40 Compare July 5, 2020 22:12
@MaskRay MaskRay force-pushed the master branch 4 times, most recently from a33231a to cb08df4 Compare July 20, 2020 00:03
@MaskRay MaskRay force-pushed the master branch 3 times, most recently from ee29996 to feb153a Compare December 20, 2020 03:01
@MaskRay MaskRay force-pushed the master branch 9 times, most recently from 94ba2b3 to c018bce Compare May 9, 2021 18:34
@MaskRay MaskRay force-pushed the master branch 3 times, most recently from db890d4 to cc13ced Compare November 6, 2024 05:57
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.

2 participants