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

use hierarchical code action kinds #1326

Closed
Trass3r opened this issue Oct 6, 2022 · 4 comments
Closed

use hierarchical code action kinds #1326

Trass3r opened this issue Oct 6, 2022 · 4 comments
Labels
enhancement New feature or request

Comments

@Trass3r
Copy link

Trass3r commented Oct 6, 2022

https://github.com/llvm/llvm-project/blob/7438df99604233bf9e319875adcbd97aae608520/clang-tools-extra/clangd/ClangdLSPServer.cpp#L611-L614

I guess clangd would need to use the finer-grained
https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionKind
and maybe also other new fields like disabled or triggerKind to enable the new
https://code.visualstudio.com/updates/v1_71#_new-code-action-control
https://code.visualstudio.com/updates/v1_72#_new-code-action-groups

@Trass3r Trass3r added the enhancement New feature or request label Oct 6, 2022
@Trass3r
Copy link
Author

Trass3r commented Oct 7, 2022

PoC: Trass3r/llvm-project@a43c9d4
Displaying disabled actions along with a reason probably requires some refactoring (Tweak::prepare?).

@kadircet
Copy link
Member

thanks for bringing this up and the PoC implementation. my main worry is around the server/client negotiation between supported code action kinds. according to spec, client announces a set of supported kinds and the ones outside this range should be handled gracefully (when announced), exact wording:

codeActionKind: {

			/**
			 * The code action kind values the client supports. When this
			 * property exists the client also guarantees that it will
			 * handle values outside its set gracefully and falls back
			 * to a default value when unknown.
			 */
			valueSet: [CodeActionKind](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#codeActionKind)[];
		};

so we actually need to figure out a way to downgrade them to basic versions again if client doesn't support the specific kinds for whatever reason. which sounds like a bunch of complexity. moreover we also need to figure out the categorization for every new tweak we want to introduce.

so in the face of all of these cons, i am actually failing to see what extra benefit this does bring us. a nicer grouping of actions into categories and different icons, probably only on VSCode. so i am afraid all that complexity might not be worth it, at least for clangd as we don't really have that many code actions available at a given selection anyway (as of writing).

as for providing disabled code actions, according to spec, they're again shown on only a few places and it would require changes to current infra (which isn't a huge amount of work) but extra mental load for writing new tweaks and also updating the existing ones. i can see how useful it can be when people are searching for a certain type of code action, but i don't think this is a big use case today either to justify all that mental load.

so i'd rather put this on hold until we get more traction around code actions, both on clangd's and other editor's priority list.
WDYT?

@Trass3r
Copy link
Author

Trass3r commented Nov 14, 2022

Ok fair enough.

@kadircet
Copy link
Member

sorry for taking a while to get to it, and only coming with bad news. i hope the code actions parts of LSP gets some traction in the near future and we will have more reasons to provide better support here. closing the issue until then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants