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

Previous Next

Full log

Message #77 received at 60338 <at> debbugs.gnu.org (full text, mbox):

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

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."

Previous Next

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