Subject: bug#63372: [PATCH] Add variable: eglot-apply-text-edits-function

From: João Távora <[email-masked]>
Date: Mon, 8 May 2023 17:33:06 +0100

On Mon, May 8, 2023 at 3:30 PM Felician Nemeth
<[email-masked]> wrote:
>
> In https://lists.gnu.org/archive/html/emacs-devel/2023-05/msg00173.html
> Eli Zaretskii <[email-masked]> writes:
>
> >> From: Felician Nemeth <[email-masked]>
> >> Cc: [email-masked],  [email-masked]
> >> Date: Sat, 06 May 2023 17:17:14 +0200
> >>
> >> 3. Eglot uses the eglot--apply-text-edits defun to apply server
> >>    initiated edits.  There is an extension that allows the server to
> >>    send the edits in a different format (snippet-text-edits).
> >>
> >>    Eglot-x puts an advise on eglot--apply-text-edits to check the format
> >>    of the edits and act accordingly.
> >>
> >>    I don't know how to avoid this advice.
> >
> > Some hook or function variable, perhaps?  If they don't exist, perhaps
> > they could be added?
>
> I've attached a patch with my first attempt at this.  João, what do you
> think of this approach?

It could work.  But I think   Some comments

? +(defvar eglot-apply-text-edits-function #'eglot--apply-lsp-text-edits

eglot--apply-lsp-text-edits should in theory be external, because it
is something pluggable on and off.

IME these things also tend lend themselves to "multiple handlers"
in the future, so maybe a generalized "special" hook (ending in
"functions", plural) is better.  Then some `run-hook-wrapped`
would iterate through it.

But then, I wonder, since most of Eglot's API is already CLOS
based, isn't a generic function best?  A generic function can
have :around methods, and if we follow the convention of
passing the server as the first argument, third parties
can make server-specific extensions.

Also, in some CLOS versions (not eieio.el's yet) there
are even method combinations that simulate hooks.

> +(cl-defun eglot--apply-lsp-text-edits (edits)
> +  "Apply EDITS for current buffer."
>    (atomic-change-group
>     (let* ((change-group (prepare-change-group))

Won't every "apply edit" function need this as boilerplate to
guarantee undo-stability?  Likely this should be popped to
around the call to the generic function.

> (Independently of this issue, maybe a configurable
> apply-workspace-edit-function would be useful as well.  One alternative
> implementation of the current eglot--apply-workspace-edit could be to
> apply all edits without asking for confirmation and then show a
> `vc-diff'-like interface allowing the user to revert/accept all of the
> changes with a single keystroke.)

If this is truly independent, request it in a different bug report
(or a different thread within this bug) with a different patch.
If it is not  independent, let's focus on the infrastructure functionality
first.

João