Close #393: eglot-completing-read

[theothornhill]

This is a first try on the completion-read function. Highly inspired from lsp-mode, maybe this is something we can use?

[joaotavora]

@theothornhill this is not exempt from copyright paperwork, because:

1. It is not a "tiny" or "trivial" patch
2. It is "highly inspired" by lsp-mode, which means it might be a derivative work.

Probably 2 is not a problem since Yasnippet also has that implementation, much earlier than lsp-mode and it is simple enough (and Yasnippet is the FSF's already).

Anyway, I will review this when I have time.

[theothornhill]

I understand. I will clean up the PR, and let you have a look. FYI, the implementation inside eglot-code-actions is more of a proof-of-concept. It is a very simple implementation still

[theothornhill]

which means it might be a derivative work.

Inspiration is mostly the last line though, and it seems like a quite general thing, but I don’t know much about these things

[theothornhill]

I added an extra commit with some more refactorings and functionality. I feel this helps readability of eglot-code-actions, yet unsure if deviates too much from current coding style

[joaotavora]

@theothornhill before you go further, let me tell you that even before that newest commit this sounds like a lot of code to solve a problem that I don't really remember any more: you want to select LSP code actions in text mode exclusively, right?

[theothornhill]

Well, this commit allows us to use the completion framework of choice to select and navigate code actions. Right now we have to use tmm, but I believe we should be able to use ido/helm/ivy/vanilla. All of them hook into completing-read.

As far as a lot of code goes, it’s mostly extracted from the existing function. It can easily be added back.

I believe we could change this so that it only works for completing code actions, but I think some sort of lsp-response-completing-read would simplify a lot of the code in Eglot

[joaotavora]

but I think some sort of lsp-response-completing-read would simplify a lot of the code in Eglot

What other code besides LSP code-action selections?

[theothornhill]

I think workspace allows for returning available workspaces, and I assume it is returned through jsonrpc, and as such could benefit from creating alist and completing them. Not sure if it is implemented yet in eglot though

[theothornhill]

I like the minimalism of eglot - it’s why I use it. However I think normal completing read offers equivalent functionality to tmm, but in addition gives people who use ivy/helm a unified experience

[joaotavora]

I'm OK with adding completing-read somewhere in the code, but you seem to be touching too much, and I don't see a use for it outside LSP code actions. I can't get a server to trigger code actions right now (all I have handy is clangd and pyls), so I can't show you what I absolutely suspect is a much simpler implementation for this, like this

diff --git a/eglot.el b/eglot.el
index 4c0160a..fd43ce2 100644
--- a/eglot.el
+++ b/eglot.el
@@ -2461,12 +2461,10 @@ If SKIP-SIGNATURE, don't try to send textDocument/signatureHelp."
          (menu `("Eglot code actions:" ("dummy" ,@menu-items)))
          (action (if (listp last-nonmenu-event)
                      (x-popup-menu last-nonmenu-event menu)
-                   (let ((never-mind (gensym)) retval)
-                     (setcdr (cadr menu)
-                             (cons `("never mind..." . ,never-mind) (cdadr menu)))
-                     (if (eq (setq retval (tmm-prompt menu)) never-mind)
-                         (keyboard-quit)
-                       retval)))))
+                   ;; maybe missing a `cdr' or `cadr' or sth like that here
+                   (assoc (completing-read "[eglot] Action? "
+                                           menu-items nil 'required)
+                          menu-items))))
     (eglot--dcase action
         (((Command) command arguments)
          (eglot-execute-command server (intern command) arguments))

[joaotavora]

Can you start from that implementation and test?

[theothornhill]

Can you start from that implementation and test?

This version should work fine for completing-read of code actions

[joaotavora]

Thanks! Now if you additionally also think that auto-confirming a single option is a good idea, work on a separate pull request for that, according to what we already discussed.

[nemethf]

It was possible to select a non-default action with just one keystroke with the old tmm-based solution. This is now more complicated with the new completing-read version with its default settings. Is there a completion package that provides easy selection? (I haven't tried all of them.) Thank you.

[nemethf]

Hm. Or is it possible to use tmm as a completing-read function?

[joaotavora]

Is there a completion package that provides easy selection? (I haven't tried all of them.)

I haven't tried them, but Ivy and Helm are pretty popular options. I personally use fido-mode in the Emacs 27 branch, you can convince to add, say, numbered shortcuts to it :-)

[joaotavora]

Hm. Or is it possible to use tmm as a completing-read function?

That would be interesting, I think.

[nemethf]

Thanks. I've unfortunately accumulated a decent backlog, but I'm going to look at it.

[nemethf]

Hm. Or is it possible to use tmm as a completing-read function?

That would be interesting, I think.

It wasn't possible out of the box, but I took a stab at it: https://github.com/nemethf/single-key-completion

I'd like to test it with Eglot. Do you happen to know an example where the server returns more than one code-action? Thanks.

[theothornhill]

Using typescript-language-server and create-react-app, you can try to create a new class/hook, then import to App.js. Usually gives me import as a code action and move to new file. Not sure how minimal you would consider this to be, but I believe it’s reproducible at least :)

Typescript-language-server gives a lot of code actions all over, so should be easy enough to get it