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 doesn't handle code action refactor.extract.* #6702

Closed
sigmaSd opened this issue Apr 11, 2023 · 6 comments
Closed

lsp doesn't handle code action refactor.extract.* #6702

sigmaSd opened this issue Apr 11, 2023 · 6 comments
Labels
C-enhancement Category: Improvements

Comments

@sigmaSd
Copy link
Contributor

sigmaSd commented Apr 11, 2023

helix can handle currently code action like refactor.extract (used by rust analyzer for example) but the spec says that kind in code action are a hierarchical list of identifiers separated by ., this is not handled by helix

example deno lsp which uses this https://github.com/denoland/deno/blob/015eb4ef9c99526486e372489dcc50feb7b39ea6/cli/lsp/refactor.rs#L23

log

2023-04-11T05:39:28.840 helix_term::commands::lsp [DEBUG] code action: CodeAction { title: "Extract to function in module scope", kind: Some(CodeActionKind("refactor.extract.function")), diagnostics: None, edit: None, command: None, is_preferred: Some(false), disabled: None, data: Some(Object {"actionName": String("function_scope_0"), "range": Object {"end": Object {"character": Number(0), "line": Number(2)}, "start": Object {"character": Number(0), "line": Number(0)}}, "refactorName": String("Extract Symbol"), "specifier": String("file:///home/mrcool/dev/deno/lab/tmp/oXf/a.ts")}) }
@sigmaSd sigmaSd added the C-enhancement Category: Improvements label Apr 11, 2023
@the-mikedavis
Copy link
Member

This could probably re-use the tree UI component (#5768)

@pascalkuthe
Copy link
Member

pascalkuthe commented Apr 14, 2023

Helix does support the kind field of code actions and we do treat them as hierarchal. This was implemented in #4134.
Code actions are sorted by their hierarchal scope. The behavior very closely matches VSCode (and refactor.extract is specifically supported) and respects the hierarchal nature of the code action kind.

The only difference to VSCode is that they show headings when a different kind starts while we do not. I don't think we should display these headings, they serve no real purpose and make the code action popup more drawn out/harder to scan.

The LSP standard says nowhere that these should be displayed in a hierarchical manner (and arguably with VSCode being the reference implementation that was never intended). It just lets the client know how this information can be interpreted. The only other place where the code action kind has any application is when filtering code actions in some form. For example in #6486. However, currently, no such config option exists in helix yet so I don't see anything missing from helix.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Apr 14, 2023

what I mean but doesn't handle it, is that helix doesn't seem to respond to it at all (it does show as an option in the code action menu but when I click nothing happens)

To reproduce:

  • install deno
  • add this config
[[language]]
name = "typescript"
file-types = ["ts","js","tsx","jsx"]
auto-format = true
[language.language-server]
command = "deno"
args = ["lsp"]
  • open a javascript file , write something like let a = 4, select it , and the run the extract to function code action

Expected behavior:
It gets extracted to a function
Actual behavior
nothing happens

Note: same thing works with rust-analyzer but in this case it uses refactor.extract and helix seems to understand it

log after clicking the code action (uses refactor.extract.function):

2023-04-11T05:39:28.840 helix_term::commands::lsp [DEBUG] code action: CodeAction { title: "Extract to function in module scope", kind: Some(CodeActionKind("refactor.extract.function")), diagnostics: None, edit: None, command: None, is_preferred: Some(false), disabled: None, data: Some(Object {"actionName": String("function_scope_0"), "range": Object {"end": Object {"character": Number(0), "line": Number(2)}, "start": Object {"character": Number(0), "line": Number(0)}}, "refactorName": String("Extract Symbol"), "specifier": String("file:///home/mrcool/dev/deno/lab/tmp/oXf/a.ts")}) }

But no answer from helix

helix 23.03

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Apr 14, 2023

This could probably re-use the tree UI component (#5768)

Ah now I see, I guess I was not clear at all , this is not what I meant, see the above comment

@gabydd
Copy link
Member

gabydd commented Apr 15, 2023

is this #5118? seems like there is no edit so I assume it's because it is using code action resolve.

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Apr 15, 2023

Yep you're right I forget about this

@sigmaSd sigmaSd closed this as completed Apr 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

4 participants