Implement on-type-formatting support

[nemethf]

• eglot.el (eglot--on-type-formatting-chars): New local var, set
  and unset ...
  (eglot--managed-mode): ... here based on server's
  documentOnTypeFormatting capability, ...
  (eglot--post-self-insert-hook): and used here to call eglot-format
  for on-type-formatting characters.
  (eglot-format): Add new optional argument character to request
  :textDocument/onTypeFormatting.

• eglot-tests.el (eglot--simulate-key-event): New helper defun.
  (rust-on-type-formatting): New test.

[joaotavora]

Thanks very much! Looks good, but please consider my comments and changes.

[joaotavora]

This docstring needs an overhaul, it should become something more generic like Do Eglot things after a character is inserted., maybe with better imagination.

[joaotavora]

Say the type of CHARACTER, i.e. that it is a character (or a one-character string). Perhaps name it ON-TYPE-FORMAT.

[joaotavora]

There is ert-simulate-keys in ert-x.el. A hack it itself seems less hacky than this (and not our responsibility ;-) ) Doesn't it work? If it doesn't then fine to add this substitute.

[nemethf]

ert-simulate-keys is not in Emacs 27.1, but more importantly it does something else: it provides user input for a function if it queries the user input (eg. read-from-minibuffer) during its execution. The goal of eglot--simulate-key-event is initiate a command-loop for a given keypress. I think the whole thing can be replaced with (setq last-input-event ?.) (self-insert-command 1 ?.) as that also calls post-self-insert-hooks. But then some part of the implementation becomes part of the test. An earlier iteration of this PR redefined keybindings for the shake of execution speed. eglot--simulate-key-event worked with that implementation as well.

Shall I (a) change the test to use (setq last-input-event ?.) (self-insert-command 1 ?.), or (b) extend the documentation of eglot--simulate-key-event?

[joaotavora]

Actually, I see that even the new server slot isn't needed. This is already in the server as "capabilities". So just grab it from there. I don't think processing required is extremely more onerous. At least not when compared to the performance hit we're already incurring in when calling jsonrpc-request from the post-self-insert-hook.

So this should simplify the code a lot.

[nemethf]

Actually, I see that even the new server slot isn't needed. This is already in the server as "capabilities". So just grab it from there. I don't think processing required is extremely more onerous. At least not when compared to the performance hit we're already incurring in when calling jsonrpc-request from the post-self-insert-hook.

So this should simplify the code a lot.

The goal of precomputing eglot--on-type-formatting-chars is make post-self-insert-hook run fast for chars that are not trigger chars for on-type-formatting. Shall I (a) add this to the documentation of eglot--on-type-formatting-chars, or (b) simplify the code and calculate the list every time the user presses a key?

I'll address the rest of your comments once I see clear with these two questions.

Thanks for the review.

[joaotavora]

Please use (b) and use cons to make list construction easier. João

…

On Mon, Mar 28, 2022, 07:43 Felicián Németh ***@***.***> wrote: Actually, I see that even the new server slot isn't needed. This is already in the server as "capabilities". So just grab it from there. I don't think processing required is extremely more onerous. At least not when compared to the performance hit we're already incurring in when calling jsonrpc-request from the post-self-insert-hook. So this should simplify the code a lot. The goal of precomputing eglot--on-type-formatting-chars is make post-self-insert-hook run fast for chars that are not trigger chars for on-type-formatting. Shall I (a) add this to the documentation of eglot--on-type-formatting-chars, or (b) simplify the code and calculate the list every time the user presses a key?

[joaotavora]

won't this aref 💥 if the server is not capable of this?

[nemethf]

I've force-pushed a new version addressing your concerns including the aref 💥.

It turned out the experimental/serverStatus is just as slow as waiting for the first diagnostic notification.

[joaotavora]

I took the liberty of making some minor tweaks and pushed this. Let's keep eyes peeled for any problems (performance, 💥, etc), though I think this is pretty sane.

[nemethf]

Thanks!