GNU bug report logs - #60338
[PATCH] Add option to present server changes as diffs

Previous Next

Full log

View this message in rfc822 format

From: João Távora <joaotavora <at> gmail.com>
To: Eshel Yaron <me <at> eshelyaron.com>
Cc: dmitry <at> gutov.dev, Philip Kaludercic <philipk <at> posteo.net>, 60338 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: bug#60338: [PATCH] Add option to present server changes as diffs
Date: Fri, 1 Sep 2023 23:01:37 +0100

OK, Eshel

I think I fixed all these issues in the latest
fdf6c164efd0bb467d0d46460161c146e955a48c which I just
pushed to master.  Please have a look.

Thanks,
João

On Fri, Sep 1, 2023 at 10:13 PM Eshel Yaron <me <at> eshelyaron.com> wrote:
>
> João Távora <joaotavora <at> gmail.com> writes:
>
> > I've finally pushed this to master, after working on mostly on the user
> > confirmation logic.  I deprecated `eglot-confirm-server-initiated-edits'
> > (had crap semantics anyway) and replaced it with a shiny new
> > 'eglot-confirm-server-edits', with a simpler name but much more powerful
> > (see its docstring).
> >
> [...]
> >
> > Please have a look, give it some testing, and tell me if I missed
> > anything.
>
> Great stuff!  One issue I came across is with unsaved buffers.
> Here's a recipe:
>
> With emacs -Q, set `eglot-confirm-server-edits` to `diff`.  In a fresh
> git repository, create a new file `foo.go` and insert the following:
>
> --8<---------------cut here---------------start------------->8---
> package baz
>
> type Foobar interface {}
> --8<---------------cut here---------------end--------------->8---
>
> Now save the file, and do `M-x go-ts-mode` followed by `M-x eglot` to
> connect to an LSP server (`gopls` in my case).  Without saving the
> buffer, insert `type Baz interface {}` after the last line:
>
> --8<---------------cut here---------------start------------->8---
> package baz
>
> type Foobar interface {}
> type Baz interface {}
> --8<---------------cut here---------------end--------------->8---
>
> With point on `Baz`, do `M-x eglot-rename RET Spam RET`.  Emacs displays
> the following diff in *EGLOT proposed server changes*:
>
> --8<---------------cut here---------------start------------->8---
> diff -u /Users/eshelyaron/tmp/bug/foo.go /var/folders/q1/h0vdt_hx00zgbv3bpyx2vdr00000gp/T/buffer-content-sgzkbC
> --- /Users/eshelyaron/tmp/bug/foo.go    2023-09-01 20:55:02
> +++ /var/folders/q1/h0vdt_hx00zgbv3bpyx2vdr00000gp/T/buffer-content-sgzkbC      2023-09-01 20:55:07
> @@ -1,3 +1,4 @@
>  package baz
>
>  type Foobar interface {}
> +Spam
> \ No newline at end of file
>
> Diff finished.  Fri Sep  1 20:55:08 2023
> --8<---------------cut here---------------end--------------->8---
>
> We clearly get an incorrect patch. It seems like the patch is produced
> by comparing the modified version of the buffer to the visited file,
> instead of comparing it to the current buffer contents.
>
> Another, smaller issue is that while `eglot-rename` is creating the
> patch it prints messages along the lines of:
>
>     [eglot] applying 1 edits to `*temp*-207900'
>
> Which feels less appropriate in these settings.
>
> Lastly, I have an improvement suggestion for the `:type` of
> `eglot-confirm-server-edits` to make it nicer to deal with via Custom:
>
> diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
> index 8d95019c3ed..afb99e4b9ca 100644
> --- a/lisp/progmodes/eglot.el
> +++ b/lisp/progmodes/eglot.el
> @@ -413,12 +413,17 @@ eglot-confirm-server-edits
>  `eglot-code-actions', `eglot-code-action-quickfix', etc.  ACTION
>  is one of the symbols described above.  The value `t' for COMMAND
>  is accepted and its ACTION is the default value."
> -  :type '(choice (const :tag "Use diff" diff)
> +  :type (let ((basic-choices
> +               '((const :tag "Use diff" diff)
>                   (const :tag "Summarize and prompt" summary)
>                   (const :tag "Maybe use diff" maybe-diff)
>                   (const :tag "Maybe summarize and prompt" maybe-summary)
> -                 (const :tag "Don't confirm" nil)
> -                 (alist :tag "Per-command alist")))
> +                 (const :tag "Don't confirm" nil))))
> +          `(choice ,@basic-choices
> +                   (alist :tag "Per-command alist"
> +                          :key-type (choice (function :tag "Command")
> +                                            (const :tag "Default" t))
> +                          :value-type (choice . ,basic-choices)))))
>
>  (defcustom eglot-extend-to-xref nil
>    "If non-nil, activate Eglot in cross-referenced non-project files."



-- 
João Távora

Previous Next

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