-
Notifications
You must be signed in to change notification settings - Fork 199
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
Keybindings for eglot-code-actions #411
Comments
I think the default is set to the first item. So you can just press RET in Or are you talking about "auto-confirming a single option"? |
Yes, I suppose that could work. However, it would better if I could bind import organisation explicitly, like I can for I recently migrated from lsp-mode, where organising imports is a dedication function: https://github.com/emacs-lsp/lsp-mode/blob/c7dc0ed22a3fc3c759809b7e9d8a62be2311b31c/lsp-mode.el#L4490 |
elgot-format almost directly calls the LSP method textDocument/rangeFormatting. There is no such method for organizing imports. Hm. Do you want to get code actions for the current point restricted to a given CodeActionKind? That wouldn't be hard to do. |
s/get/apply, but yes! That would be the general solution. 🙂 |
Can you show me a minimal example where the "auto-confirmation of the single option" is different from the above? I.e., where there are more than one code actions for a given point and exactly one of them has source.organizeImports as its CodeActionKind. |
I've never seen any other option presented. I only use |
clangd gives two code-actions for the following snippet at #include <iostream>
using namespace std;
int func_a(int arg) {
return arg + 1;
}
int main()
{
cout << "Hello, World!";
int a = func_b(5);
return 0;
} server's reply to textDocument/codeAction: (:id 2 :result
[(:diagnostics
[(:code "undeclared_var_use_suggest" :message "Use of undeclared identifier 'func_b'; did you mean 'func_a'? (fix available)\n\na.cpp:8:5: note: 'func_a' declared here" :range
(:end
(:character 16 :line 14)
:start
(:character 10 :line 14))
:severity 1 :source "clang")]
:edit
(:changes
(:file:///home/nemethf/src/eglot/ccls-test/a\.cpp
[(:newText "func_a" :range
(:end
(:character 16 :line 14)
:start
(:character 10 :line 14)))]))
:kind "quickfix" :title "change 'func_b' to 'func_a'")
(:command
(:arguments
[(:file "file:///tmp/proxy-cache/89f52e8f-d7c0-4b59-a535-175b11316b39/home/nemethf/src/eglot/ccls-test/a.cpp" :selection
(:end
(:character 16 :line 14)
:start
(:character 10 :line 14))
:tweakID "ExtractVariable")]
:command "clangd.applyTweak" :title "Extract subexpression to variable")
:kind "refactor" :title "Extract subexpression to variable")]
:jsonrpc "2.0") If we want to provide a functionality that restricts the possible code actions to a given kind ("quickfix", "refactor", or "source.organizeImports") and applies a single candidate without conformation, then it would be good to inform the user what's happening in detail. When text edits arrive in the textDocument/codeAction it's easy to provide information to the user, because the title is available ("change 'func_b' to 'func_a" in the example above). However, when the code action is a command (the second action in the example above), then the client sends an workspace/executeCommand, and the server later request a workspace/applyEdit, in which the title is not given ("Extract subexpression to variable"). I see no easy way to correlate the workspace/applyEdit with the workspace/executeCommand. (But selecting the second choice sometimes results in a bug, so I need to investigate this further... It might be the result of my special setup, I run the servers in a docker container. ) |
If eglot-confirm-server-initiated-edits was nil and we had "auto-confirming a single option", then users weren't informed about the details of edits during eglot-code-actions. This PR helps in this regard. The server's reply to a textDocument/codeAction may contain workspace edits with titles, or commands with titles. However, the client needs to send the command the sever, and the server replies something, and later it sends a workspace edit without a title. Therefore, the new variable `eglot--last-title' caches this information.
I haven't worked on the "auto-confirming a single option", but using the single key completion backend has similar effects. Moreover, I created a new branch wip-411-advertise-title that hopefully helps to understand the progress of automatic edits when @mpolden, would you like to try it out and give feedback about the modified user interface? |
How is that change relevant to this issue? For my use case (organize imports), I'm never asked to confirm server edits ( |
The server communicates "import organization" through code actions. So even if this doesn't improve your use case, it definitely alters it. |
share my implemention of
|
I think part the confusion here comes from the list of “possible code actions”. As I see it, the use-case for this feature request is for the user to initiate a single, unambiguous, caller-specified code action (such as Given that, I don't think there is a strong need to “inform the user what's happening”: they know what key they pressed and they can easily find out what function that key is bound to, or else figure it out from the Emacs stack trace if need be. Informing the user about what's going on seems nice to have, but a narrower, more targeted fix should not require that. (The |
@jixiuf, your solution doesn't work reliably for me. If try to save the file soon after loading the file and making an edit, the
Eglot event log
`gopls` event log
|
I have added a self-contained ELISP reproducer for the above failure mode at https://github.com/bcmills/eglot-issue-411. To run it, clone the repo and execute |
I would also be interested to see this |
The action kind can be passed as argument: (defun eglot-code-action-kind (action-kind)
"Offer to execute the ACTION-KIND code action."
(interactive "sAction kind: ")
(unless (eglot--server-capable :codeActionProvider)
(eglot--error "Server can't execute code actions!"))
(let* ((server (eglot--current-server-or-lose))
(actions (jsonrpc-request
server
:textDocument/codeAction
(list :textDocument (eglot--TextDocumentIdentifier))))
(action (cl-find-if
(jsonrpc-lambda (&key kind &allow-other-keys)
(string-equal kind action-kind))
actions)))
(when action
(eglot--dcase action
(((Command) command arguments)
(eglot-execute-command server (intern command) arguments))
(((CodeAction) edit command)
(when edit (eglot--apply-workspace-edit edit))
(when command
(eglot--dbind ((Command) command arguments) command
(eglot-execute-command server (intern command) arguments))))))))
(defun eglot-organize-imports ()
"Offer to execute the source.organizeImports code action."
(interactive)
(eglot-code-action-kind "source.organizeImports")) (based on #411 (comment)) This will allow to create shortcuts easily. WDYT? |
Thanks for this snippet. |
I had success with asking for actions like this:
|
Looks almost like Lines 2538 to 2546 in 0c4daa4
Maybe the better way would be to extract the actions retrieval code from the |
Or make the |
Yes, it's similar. Though with fixed start and end positions spawning the whole document and for the server the instruction to only include the requested CodeAction, if available. |
@joaotavora Do you think those IMO replacing them with |
The organize Imports is working for me on Metals only like this: (defun eglot-organize-imports ()
"Offer to execute code actions `source.organizeImports'."
(interactive)
(unless (eglot--server-capable :codeActionProvider)
(eglot--error "Server can't execute code actions!"))
(let* ((server (eglot--current-server-or-lose))
(actions
(jsonrpc-request
server
:textDocument/codeAction
(list :textDocument (eglot--TextDocumentIdentifier)
:range (list :start (eglot--pos-to-lsp-position 0)
:end (eglot--pos-to-lsp-position (point-max)))
:context (list :diagnostics []
:only ["source.organizeImports"]))))
(action (car (mapcar (jsonrpc-lambda (&rest all &allow-other-keys)
all)
actions))))
(message "Action: %s" action)
(when action
(eglot--dcase action
(((CodeAction) edit command)
(when edit (eglot--apply-workspace-edit edit))
(when command
(eglot--dbind ((Command) command arguments) command
(eglot-execute-command server (intern command) arguments)))))))) Not sure if this I think for general code-actions the begin and end range is useful and should be kept. |
@muffinmad , I didn't follow this very closely, but I think you're on to something by adding optional arguments to
As you can read the current code, an interactive call sets at least beg, meaning "I want code actions right here". And if there's a region both are used. So I wouldn't get rid of them. But your filtering can be implemented as an extra argument, why not? If the dumb optionality of Elisp
In my opinion, replacing is bad, but adding this option is very good. You can even make Thanks everybody, I like where this is going. |
That sounds like a good idea. |
I'm not proposing to get rid of the Lines 1469 to 1471 in 0c4daa4
Adding another optional argument will look like this: (defun eglot-code-actions (beg &optional end action-kind) So there are no simply way to just call
I'll take a look at
Ok, let's try this approach! |
Eglot doesn't even call it, as far as I can read in the code. It is currently meant primarily for interactive use.
But are you saying that adding |
Yes. It's called from mouse clicks, with the proper range (beg, end) of the flymake diagnostic in question. A third parameter might be OK, when optional. Obviously it should stay unset when called from mouse click. |
Make `eglot-code-actions` accept `action-kind` argument. If there are only one action of that kind, apply it. This will allow to create actions shortcuts like (defun eglot-code-action-organize-imports () (interactive) (eglot-code-actions nil nil "source.organizeImports")) * eglot.el (eglot-code-actions): Accept new argument `action-kind`. Execute the only action of that kind.
Make `eglot-code-actions` accept `action-kind` argument. If there are only one action of that kind, apply it. This will allow to create actions shortcuts like (defun eglot-code-action-organize-imports () (interactive) (eglot-code-actions nil nil "source.organizeImports")) * eglot.el (eglot-code-actions): Accept new argument `action-kind`. Execute the only action of that kind.
…e actions Make eglot-code-actions accept a new action-kind argument. If there is only one action of that kind, apply it. This allows us to create actions shortcuts like eglot-code-action-organize-imports, etc. * eglot.el (eglot-code-actions): Accept new argument action-kind. (eglot--code-action): New function-defining helper macro. (eglot-code-action-organize-imports) (eglot-code-action-extract) (eglot-code-action-inline) (eglot-code-action-rewrite) (eglot-code-action-quickfix): New commands. * README.md: Mention new feature. * NEWS.md: Mention new feature. Co-authored-by: João Távora <[email protected]>
…ed code actions See also joaotavora/eglot#598. Make eglot-code-actions accept a new action-kind argument. If there is only one action of that kind, apply it. This allows us to create actions shortcuts like eglot-code-action-organize-imports, etc. * eglot.el (eglot-code-actions): Accept new argument action-kind. (eglot--code-action): New function-defining helper macro. (eglot-code-action-organize-imports) (eglot-code-action-extract) (eglot-code-action-inline) (eglot-code-action-rewrite) (eglot-code-action-quickfix): New commands. * README.md: Mention new feature. * NEWS.md: Mention new feature. Co-authored-by: João Távora <[email protected]>
…ed code actions See also joaotavora/eglot#598. Make eglot-code-actions accept a new action-kind argument. If there is only one action of that kind, apply it. This allows us to create actions shortcuts like eglot-code-action-organize-imports, etc. * eglot.el (eglot-code-actions): Accept new argument action-kind. (eglot--code-action): New function-defining helper macro. (eglot-code-action-organize-imports) (eglot-code-action-extract) (eglot-code-action-inline) (eglot-code-action-rewrite) (eglot-code-action-quickfix): New commands. * README.md: Mention new feature. * NEWS.md: Mention new feature. Co-authored-by: João Távora <[email protected]>
See also #598. Make eglot-code-actions accept a new action-kind argument. If there is only one action of that kind, apply it. This allows us to create actions shortcuts like eglot-code-action-organize-imports, etc. * eglot.el (eglot-code-actions): Accept new argument action-kind. (eglot--code-action): New function-defining helper macro. (eglot-code-action-organize-imports) (eglot-code-action-extract) (eglot-code-action-inline) (eglot-code-action-rewrite) (eglot-code-action-quickfix): New commands. * README.md: Mention new feature. * NEWS.md: Mention new feature. Co-authored-by: João Távora <[email protected]> #411: joaotavora/eglot#411 #598: joaotavora/eglot#598
See also joaotavora/eglot#598. Make eglot-code-actions accept a new action-kind argument. If there is only one action of that kind, apply it. This allows us to create actions shortcuts like eglot-code-action-organize-imports, etc. * eglot.el (eglot-code-actions): Accept new argument action-kind. (eglot--code-action): New function-defining helper macro. (eglot-code-action-organize-imports) (eglot-code-action-extract) (eglot-code-action-inline) (eglot-code-action-rewrite) (eglot-code-action-quickfix): New commands. * README.md: Mention new feature. * NEWS.md: Mention new feature. Co-authored-by: João Távora <[email protected]> GitHub-reference: close joaotavora/eglot#411
Is it possible to bind a key to a code action?
When writing Go it's very useful to have a keybinding for fixing imports. E.g. a keybinding for the first action offered by
M-x eglot-code-actions
.The text was updated successfully, but these errors were encountered: