GNU bug report logs - #63372
[PATCH] Add variable: eglot-apply-text-edits-function

Previous Next

Full log

View this message in rfc822 format

From: João Távora <joaotavora <at> gmail.com>
To: Felician Nemeth <felician.nemeth <at> gmail.com>
Cc: 63372 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: bug#63372: [PATCH] Add variable: eglot-apply-text-edits-function
Date: Mon, 8 May 2023 17:33:06 +0100

On Mon, May 8, 2023 at 3:30 PM Felician Nemeth
<felician.nemeth <at> gmail.com> wrote:
>
> In https://lists.gnu.org/archive/html/emacs-devel/2023-05/msg00173.html
> Eli Zaretskii <eliz <at> gnu.org> writes:
>
> >> From: Felician Nemeth <felician.nemeth <at> gmail.com>
> >> Cc: joaotavora <at> gmail.com,  emacs-devel <at> gnu.org
> >> 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

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.