bug#60338: [PATCH] Add option to present server changes as diffs

[Philip Kaludercic <philipk@posteo.net>]

[-- Attachment #1: Type: text/plain, Size: 156 bytes --]


The previous patch couldn't be applied onto master anymore, so I have
adjusted it and merged the user option into 'eglot-confirm-server-initiated-edits'.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-for-LSP-edits-to-be-generated-as-diffs.patch --]
[-- Type: text/x-diff, Size: 6524 bytes --]

From 7263d821c2d217c5748965aad57df8c0311f7e95 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sun, 11 Jun 2023 23:18:12 +0200
Subject: [PATCH 1/2] Allow for LSP edits to be generated as diffs

* doc/misc/eglot.texi (Eglot Variables): Document the new behaviour.
* lisp/progmodes/eglot.el (eglot-confirm-server-initiated-edits): Add
new value 'diff'.
(eglot-execute): Check if 'eglot-confirm-server-initiated-edits' is
set to 'diff'.
(eglot--apply-workspace-edit): Respect the new 'diff' value.
(eglot-rename): Pass 'eglot-confirm-server-initiated-edits' instead of
just the value 'current-prefix-arg'.  (Bug#60338)
---
 doc/misc/eglot.texi     |  6 ++--
 lisp/progmodes/eglot.el | 62 +++++++++++++++++++++++++++++++----------
 2 files changed, 51 insertions(+), 17 deletions(-)

diff --git a/doc/misc/eglot.texi b/doc/misc/eglot.texi
index 962e6c914ce..305feda43a6 100644
--- a/doc/misc/eglot.texi
+++ b/doc/misc/eglot.texi
@@ -833,9 +833,11 @@ Eglot Variables
 @item eglot-confirm-server-initiated-edits
 Various Eglot commands and code actions result in the language server
 sending editing commands to Emacs.  If this option's value is
-non-@code{nil} (the default), Eglot will ask for confirmation before
+@code{confirm} (the default), Eglot will ask for confirmation before
 performing edits initiated by the server or edits whose scope affects
-buffers other than the one where the user initiated the request.
+buffers other than the one where the user initiated the request.  If
+set to @code{diff}, Eglot will generate and display a diff to display
+the changes.
 
 @item eglot-ignored-server-capabilities
 This variable's value is a list of language server capabilities that
diff --git a/lisp/progmodes/eglot.el b/lisp/progmodes/eglot.el
index 7d5d786dea3..9c9c3152472 100644
--- a/lisp/progmodes/eglot.el
+++ b/lisp/progmodes/eglot.el
@@ -108,6 +108,7 @@
 (require 'filenotify)
 (require 'ert)
 (require 'text-property-search nil t)
+(require 'diff)
 
 ;; These dependencies are also GNU ELPA core packages.  Because of
 ;; bug#62576, since there is a risk that M-x package-install, despite
@@ -386,9 +387,14 @@ eglot-events-buffer-size
   :type '(choice (const :tag "No limit" nil)
                  (integer :tag "Number of characters")))
 
 (defcustom eglot-confirm-server-initiated-edits 'confirm
-  "Non-nil if server-initiated edits should be confirmed with user."
+  "Control how server edits are applied.
+If set to `confirm', a prompt is presented to confirm server
+changes.  If set to `diff', a buffer will pop up with changes
+that can be applied manually.  If set to nil, modifications are
+made automatically."
   :type '(choice (const :tag "Don't show confirmation prompt" nil)
+                 (const :tag "Present as diffs in a buffer" diff)
                  (const :tag "Show confirmation prompt" confirm)))
 
 (defcustom eglot-extend-to-xref nil
@@ -740,7 +746,8 @@ eglot-execute
    (eglot--dcase action
      (((Command)) (eglot--request server :workspace/executeCommand action))
      (((CodeAction) edit command)
-      (when edit (eglot--apply-workspace-edit edit))
+      (when edit
+        (eglot--apply-workspace-edit edit (eq eglot-confirm-server-initiated-edits t)))
       (when command (eglot--request server :workspace/executeCommand command))))))
 
 (cl-defgeneric eglot-initialization-options (server)
@@ -3408,18 +3415,43 @@ eglot--apply-workspace-edit
         ;; prefer documentChanges over changes.
         (cl-loop for (uri edits) on changes by #'cddr
                  do (push (list (eglot--uri-to-path uri) edits) prepared)))
-      (if (or confirm
-              (cl-notevery #'find-buffer-visiting
-                           (mapcar #'car prepared)))
-          (unless (y-or-n-p
+      (cond
+       ((eq confirm 'diff)
+        (with-current-buffer (get-buffer-create " *Changes*")
+          (buffer-disable-undo (current-buffer))
+          (let ((buffer-read-only t))
+            (diff-mode))
+          (let ((inhibit-read-only t)
+                (target (current-buffer)))
+            (erase-buffer)
+            (pcase-dolist (`(,path ,edits ,_) prepared)
+              (with-temp-buffer
+                (let ((diff (current-buffer)))
+                  (with-temp-buffer
+                    (insert-file-contents path)
+                    (eglot--apply-text-edits edits)
+                    (diff-no-select path (current-buffer)
+                                    nil t diff))
+                  (with-current-buffer target
+                    (insert-buffer-substring diff))))))
+          (setq-local buffer-read-only t)
+          (buffer-enable-undo (current-buffer))
+          (goto-char (point-min))
+          (pop-to-buffer (current-buffer))
+          (font-lock-ensure)))
+       ((and (or (eq confirm 'confirm)
+                 (cl-find-if-not #'find-buffer-visiting prepared
+                                 :key #'car))
+             ;; Should the list of buffers be popped up temporarily in
+             ;; a buffer instead of resizing the minibufffer?
+             (not (y-or-n-p
                    (format "[eglot] Server wants to edit:\n  %s\n Proceed? "
-                           (mapconcat #'identity (mapcar #'car prepared) "\n  ")))
-            (jsonrpc-error "User canceled server edit")))
-      (cl-loop for edit in prepared
-               for (path edits version) = edit
-               do (with-current-buffer (find-file-noselect path)
-                    (eglot--apply-text-edits edits version))
-               finally (eldoc) (eglot--message "Edit successful!")))))
+                           (mapconcat #'identity (mapcar #'car prepared) "\n  ")))))
+        (jsonrpc-error "User canceled server edit"))
+       ((cl-loop for (path edits version) in prepared
+                 do (with-current-buffer (find-file-noselect path)
+                      (eglot--apply-text-edits edits version))
+                 finally (eldoc) (eglot--message "Edit successful!")))))))
 
 (defun eglot-rename (newname)
   "Rename the current symbol to NEWNAME."
@@ -3434,7 +3466,7 @@ eglot-rename
    (eglot--request (eglot--current-server-or-lose)
                    :textDocument/rename `(,@(eglot--TextDocumentPositionParams)
                                           :newName ,newname))
-   current-prefix-arg))
+   (and current-prefix-arg eglot-confirm-server-initiated-edits)))
 
 (defun eglot--region-bounds ()
   "Region bounds if active, else bounds of things at point."
-- 
2.39.2


[-- Attachment #3: Type: text/plain, Size: 183 bytes --]


In addition to that, here is a cheap command that would allow for a
patch to be applied with a single command.  I've bound it to C-c C-x,
but perhaps there is a more intuitive key?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-Add-command-to-apply-an-entire-diff-at-once.patch --]
[-- Type: text/x-diff, Size: 1351 bytes --]

From 1cf6f09318860a8232f5850c0d1ca497d8c621f9 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Sun, 11 Jun 2023 23:30:03 +0200
Subject: [PATCH 2/2] Add command to apply an entire diff at once

* lisp/vc/diff-mode.el (diff-mode-map): Bind 'diff-apply-everything'.
(diff-apply-everything): Add new command.
---
 lisp/vc/diff-mode.el | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index d776375d681..d81f0af365e 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -216,6 +216,7 @@ diff-mode-map
   "C-x 4 A" #'diff-add-change-log-entries-other-window
   ;; Misc operations.
   "C-c C-a" #'diff-apply-hunk
+  "C-c C-x" #'diff-apply-everything
   "C-c C-e" #'diff-ediff-patch
   "C-c C-n" #'diff-restrict-view
   "C-c C-s" #'diff-split-hunk
@@ -2033,6 +2034,16 @@ diff-apply-hunk
       (when diff-advance-after-apply-hunk
 	(diff-hunk-next))))))
 
+(defun diff-apply-everything ()
+  "Apply the entire diff."
+  (interactive)
+  (save-excursion
+    (goto-char (point-min))
+    (while (let ((inhibit-message t)
+                 (start (point)))
+             (diff-hunk-next)
+             (/= start (point)))
+      (diff-apply-hunk))))
 
 (defun diff-test-hunk (&optional reverse)
   "See whether it's possible to apply the current hunk.
-- 
2.39.2


[-- Attachment #5: Type: text/plain, Size: 774 bytes --]


Philip Kaludercic <philipk@posteo.net> writes:

> X-CC-Debbugs: 
>
> I'd like to propose adding an option that makes server modifications by
> Eglot less invasive.  The current behaviour is to make the changes
> directly in a buffer and open the remaining files to make the
> modifications in those as well (?).  If `eglot-use-diffs' is enabled,
> all confirmations are prepared as patches in a pop-up buffer that the
> user can review and apply at will.  To my knowledge there is no general
> `diff-apply-hunk' that will apply all the changes from a buffer, but
> that is a separate issue that can be fixed in a separate patch.
>
> (Note, I'm still testing emacs-29, so the patch was developed on that
> branch.  But it should be applied to master)

-- 
Philip Kaludercic