nemethf
commented
4 years ago I think the default is set to the first item. So you can just press RET in completing-read.
Or are you talking about "auto-confirming a single option"?
Closed mpolden closed 3 years ago
nemethf
commented
4 years ago I think the default is set to the first item. So you can just press RET in completing-read.
Or are you talking about "auto-confirming a single option"?
mpolden
commented
4 years ago Yes, I suppose that could work. However, it would better if I could bind import organisation explicitly, like I can for eglot-format.
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
nemethf
commented
4 years ago 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.
mpolden
commented
4 years ago Do you want to get code actions for the current point restricted to a given CodeActionKind?
s/get/apply, but yes! That would be the general solution. đ
nemethf
commented
4 years ago 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.
mpolden
commented
4 years ago I've never seen any other option presented. I only use gopls though.
nemethf
commented
4 years ago clangd gives two code-actions for the following snippet at func_b.
#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. )
nemethf
commented
4 years ago 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 eglot-confirm-server-initiated-edits is nil.
@mpolden, would you like to try it out and give feedback about the modified user interface?
mpolden
commented
4 years ago How is that change relevant to this issue? For my use case (organize imports), I'm never asked to confirm server edits (eglot-confirm-server-initiated-edit = 'confirm, default).
nemethf
commented
4 years ago The server communicates "import organization" through code actions. So even if this doesn't improve your use case, it definitely alters it.
jixiuf
commented
3 years ago share my implemention of eglot-organize-imports
(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))))
(action (cl-find-if
(jsonrpc-lambda (&key kind &allow-other-keys)
(string-equal kind "source.organizeImports" ))
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))))))))
(add-hook 'before-save-hook #'eglot-organize-imports 30 t)
bcmills
commented
3 years ago @nemethf
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.
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 "source.organizeImports"), which is either currently possible or not. If the action is possible, it should be performed, and if not, then it should be skipped.
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 wip-411-advertise-title branch seems like it's putting the cart before the horse.)
bcmills
commented
3 years ago @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 organizeImports action fails and Eglot emits an error:
Error: (jsonrpc-error "Edits on âmain.goâ require version 0, you have 1" (jsonrpc-error-code . 32603) (jsonrpc-error-message . "Edits on âmain.goâ require version 0, you have 1"))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 emacs -q --load script.el in the repo root (with eglot and gopls installed for the current user).
mdbergmann
commented
3 years ago I would also be interested to see this eglot-organize-imports.
Can this be a PR with only that?
muffinmad
commented
3 years ago 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 https://github.com/joaotavora/eglot/issues/411#issuecomment-730474005)
This will allow to create shortcuts easily.
WDYT?
mdbergmann
commented
3 years ago @muffinmad
Thanks for this snippet.
I'm playing with this as well.
It seems that i.e. Metals (Scala LSP server) bails out when the CodeAction request doesn't contain Range and Context parameters. But I'm unable to figure out in the spec whether those are mandatory parameters or not. In case of not it would be a problem of Metals.
mdbergmann
commented
3 years ago I had success with asking for actions like this:
(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"]))))Looks almost like https://github.com/joaotavora/eglot/blob/0c4daa40c53ede22f964d3f9ac1c20752bf97141/eglot.el#L2538-L2546
Maybe the better way would be to extract the actions retrieval code from the eglot-code-actions function.
muffinmad
commented
3 years ago Or make the eglot-code-actions function to accept the action kind to run.
mdbergmann
commented
3 years ago 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. Not sure though if this warrants such an amount of code duplication.
muffinmad
commented
3 years ago @joaotavora Do you think those beg and end arguments are commonly used? (https://github.com/joaotavora/eglot/commit/d6af4df11ad962e38d0f8ab9306e7c664ff89ac5)
IMO replacing them with action-kind can make more benefit: code action of the specific kind can be applied without the need to select the action from the list.
mdbergmann
commented
3 years ago 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 jsonrpc-lambda can be done any simpler.
But, a CodeAction request must have Range and Context parameters. In the spec only the parameters with ? are optional.
I think for general code-actions the begin and end range is useful and should be kept.
joaotavora
commented
3 years ago @muffinmad , I didn't follow this very closely, but I think you're on to something by adding optional arguments to eglot-code-actions.
@joaotavora Do you think those beg and end arguments are commonly used? (d6af4df)
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 defuns isn't pretty, feel free to use a cl-defun and &key arguments. Just remember to make the interactive spec do the right thing. This isn't 100% backward compatible, but shouldn't be terribly problematic.
IMO replacing them with action-kind can make more benefit: code action of the specific kind can be applied without the need to select the action from the list.
In my opinion, replacing is bad, but adding this option is very good. You can even make C-u M-x eglot-code-actions prompt the user for a kind of code actions to select automatically (maybe even with completion?).
Thanks everybody, I like where this is going.
mdbergmann
commented
3 years ago In my opinion, replacing is bad, but adding this option is very good. You can even make C-u M-x eglot-code-actions prompt the user for a kind of code actions to select automatically (maybe even with completion?).
That sounds like a good idea.
muffinmad
commented
3 years ago @joaotavora Do you think those beg and end arguments are commonly used? (d6af4df)
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.
I'm not proposing to get rid of the beg and end variables, but only of the beg and end arguments. I mean, does someone uses it like (eglot-code-actions 1 1)? Eglot itself doesn't use it like that, but move point before calling eglot-code-actions:
If the dumb optionality of Elisp
defunsisn't pretty,
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 (eglot-code-actions "some.action.kind").
feel free to use a
cl-defunand&keyarguments.
I'll take a look at cl-defun, thanks!
In my opinion, replacing is bad, but adding this option is very good. You can even make
C-u M-x eglot-code-actionsprompt the user for a kind of code actions to select automatically (maybe even with completion?).
Ok, let's try this approach!
joaotavora
commented
3 years ago I mean, does someone uses it like (eglot-code-actions 1 1)? Eglot itself doesn't use it like that, but move point before calling eglot-code-actions:
Eglot doesn't even call it, as far as I can read in the code. It is currently meant primarily for interactive use.
So there are no simply way to just call (eglot-code-actions "some.action.kind").
But are you saying that adding (point-min) and (point-max) is too much to type for users putting this in some hook their .emacs? Meh, I don't know. But that's why I propose :kind "some.action.kind" (i.e. the cl-defun) and we can default the other args to something logical. Of course, don't forget that it still must work interactively.
mdbergmann
commented
3 years ago It is currently meant primarily for interactive use.
Yes. It's called from mouse clicks, with the proper range (beg, end) of the flymake diagnostic in question. And that works. So this should continue to work.
A third parameter might be OK, when optional. Obviously it should stay unset when called from mouse click.
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.