Make it possible to select server to shut down from eglot-shutdown

[mkcms]

This commit makes the command eglot-shutdown ask the user for server to kill when it's called with a prefix argument. I find the current workflow for killing servers a little bit awkward (switch to project, switch to some file managed by the server, kill it). This IMO improves this workflow.

[joaotavora]

Leave the prefix argument semantics untouched and always read the server when interactively called and more than one server. Put the current server first when it exists so that single extra RET does what is expected.

We'll have to revisit this when we allow more than one server to manage a buffer, but we'll probably have to revisit a whole bunch of stuff...

BTW the reason you find it akward is probably because you are developing Eglot itself and starting and stopping a lot of servers. You probably wouldn't mind so much of you were just using Eglot regularly.

[joaotavora]

A few problems here

• apply #'append is pathologically slow for large lists because of the variable arglist of append.
  A cl-loop can iterate through hash tables cheaply and append on the go.

• there's no need to also cons a new alist or use hash-table-values

• I don't understand the need for delq nil or delete-dups (though we've been forgetting to purge the hash table of empty entries, but that's a different problem).

• We probably want to error if there are no servers and return the only server without prompting if it exists, or if it is the current server.

I came up with this lightly tested version:

(defun eglot--read-server (prompt &optional dont-if-just-the-one)
  "Read a running Eglot server from minibuffer using PROMPT.
If DONT-IF-JUST-THE-ONE and there's only one server, don't prompt
and just return it."
  (let ((servers (cl-loop for servers
                           being hash-values of eglot--servers-by-project
                           append servers)))
    (cond ((null servers)
           (eglot--error "No servers!"))
          ((or (cdr servers) (not dont-if-just-the-one))
           (let ((read (completing-read
                        prompt (mapcar #'jsonrpc-name servers)
                        nil t
                        (when-let ((current (eglot--current-server)))
                          (jsonrpc-name current)))))
             (cl-find read servers :key #'jsonrpc-name :test #'equal)))
          (t (car servers)))))

[mkcms]

apply #'append is pathologically slow for large lists because of the variable arglist of append.
A cl-loop can iterate through hash tables cheaply and append on the go.

I didn't think about that, I assumed that even with a large number of servers (I never use more than 3 myself), the delay would be unnoticeable.

I don't understand the need for delq nil or delete-dups (though we've been forgetting to purge the hash table of empty entries, but that's a different problem).

I'm prepending the value of eglot--current-server to the list, then deleting that element if it was nil. completing-read doesn't allow nil values. I'm using delete-dups because when I prepend eglot-current-server, it's still somewhere in the tail of the other list. I don't know if completing-read removes duplicates, but I use ivy, and it didn't delete them.

We probably want to error if there are no servers and return the only server without prompting if it exists, or if it is the current server.

I think it might be a bit unexpected when there is only one server and we kill it. I would have to always check how many servers there are before I call eglot-shutdown to make sure I don't kill the current server if I don't want to.

A cl-loop can iterate through hash tables cheaply and append on the go.

Thanks, I don't know 'cl very well. I will try to rewrite it, or you can push your version as mine will probably be very similar.

[joaotavora]

didn't think about that, I assumed that even with a large number of servers (I never use more than 3 myself), the delay would be unnoticeable.

If you (hash-table-count eglot-servers-by-project) the result may surprise you, but that's the bug I was referring to. Anyway, it's good practice in general to avoid applying to things whose lengths we don't control well.

I'm prepending the value of eglot--current-server to the list, then deleting that element if it was nil. completing-read doesn't allow nil values. I'm using delete-dups because when I prepend eglot-current-server, it's still somewhere in the tail of the other list. I don't know if completing-read removes duplicates, but I use ivy, and it didn't delete them.

OK, but it's not needed if you use the initial-value argument to completing-read. (What could be problematic is if you have two projects identically named)

I think it might be a bit unexpected when there is only one server and we kill it. I would have to always check how many servers there are before I call eglot-shutdown to make sure I don't kill the current server if I don't want to.

Then we can just call my version of eglot--read-server with eglot--current-server as the second argument.

Thanks, I don't know 'cl very well. I will try to rewrite it, or you can push your version as mine will probably be very similar.

OK I will push mine and I hope you don't mind the lisp pedancy.