Subject: Re: bug#65380: [PATCH] Add command to copy contents in a diff-mode buffer

From: Philip Kaludercic <[email-masked]>
Date: Tue, 20 Aug 2024 07:46:43 +0000

Juri Linkov <[email-masked]> writes:

>> I'm attaching the newest version of the patch here:
>
> Thanks, I tried it out, and it works nicely for the single hunk case.

1+

>> +*** New command 'diff-kill-ring-save'
>> +This command copies out the modified contents out of a diff, without
>> +having to apply it first.  If the selected range extends a hunk, the
>> +commands attempts to look up and copy the text between from the
>> +referenced file.
>
> I'm not sure about usefulness of the last part in multi-hunk case.
> Does someone really need to copy a huge part of the source file
> between hunks at the top and bottom?  I expected that multi-hunk case
> would copy only concatenated text of hunks, not the source file.
> But I have no strong opinion about this.  

The main scenario I had in mind was when someone modifies the beginning
and the end of a function, and I want to copy all of it at once.  I
agree that there is usually not much of a use for copying huge parts of
a source file, but we get that for free when the beginning-to-end of a
function case is handled.

>                                           My main use case will be
> to copy the text of the current hunk.  Could you please support
> this case where typing 'w' would copy the current hunk when the
> region is not activated.

Done, see below.

>> @@ -630,6 +631,22 @@ diff-end-of-hunk
>> +(defun diff-beginning-of-hunk-position (&optional try-harder)
>> +  "Call `diff-end-of-hunk' without moving.
>> +The optional argument TRY-HARDER is passed on to
>> +`diff-beginning-of-hunk'."
>> +  (save-excursion
>> +    (save-window-excursion
>> +      (diff-beginning-of-hunk try-harder))))
>> +
>> +(defun diff-end-of-hunk-position (&optional style donttrustheader)
>> +  "Call `diff-end-of-hunk' without moving.
>> +The optional arguments STYLE and DONTTRUSTHEADER are passed on to
>> +`diff-end-of-hunk'."
>> +  (save-excursion
>> +    (save-window-excursion
>> +      (diff-end-of-hunk style donttrustheader))))
>
> I don't understand why separate functions with 'save-window-excursion'
> are needed here, since all other uses of 'diff-beginning-of-hunk'
> just wrap the calls with 'save-excursion'.

You are right, it isn't needed anymore, and I have removed it.

Here is the updated patch:

>From c19e18e3a2fd8df3547ddebce0f63c1620e7d2f6 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <[email-masked]>
Date: Sat, 19 Aug 2023 11:47:54 +0200
Subject: [PATCH] Add command to copy contents in a diff-mode buffer

* lisp/vc/diff-mode.el (diff-mode-shared-map): Bind 'diff-kill-ring-save'.
(diff-mode-map): Ensure the "w" binding does not get prefixed.
(diff-kill-ring-save): Add the command.
* etc/NEWS: Mention 'diff-kill-ring-save'.  (Bug#65380)
---
 etc/NEWS             |  9 ++++++++
 lisp/vc/diff-mode.el | 51 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/etc/NEWS b/etc/NEWS
index b89a80aa14d..e493c1bb035 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -210,6 +210,15 @@ The host name for Kubernetes connections can be of kind
 used.  This overrides the setiing in 'tramp-kubernetes-namespace', if
 any.
 
+** Diff
+
+---
+*** New command 'diff-kill-ring-save'
+This command copies out the modified contents out of a diff, without
+having to apply it first.  If the selected range extends a hunk, the
+commands attempts to look up and copy the text between from the
+referenced file.
+
 
 * New Modes and Packages in Emacs 31.1
 
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 81e8b23ee33..5caf2cb5e5b 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -196,6 +196,7 @@ diff-mode-shared-map
   "RET" #'diff-goto-source
   "<mouse-2>" #'diff-goto-source
   "W" #'widen
+  "w" #'diff-kill-ring-save
   "o" #'diff-goto-source                ; other-window
   "A" #'diff-ediff-patch
   "r" #'diff-restrict-view
@@ -208,7 +209,7 @@ diff-mode-map
           ;; We want to inherit most bindings from
           ;; `diff-mode-shared-map', but not all since they may hide
           ;; useful `M-<foo>' global bindings when editing.
-          (dolist (key '("A" "r" "R" "g" "q" "W" "z"))
+          (dolist (key '("A" "r" "R" "g" "q" "W" "w" "z"))
             (keymap-set map key nil))
           map)
   ;; From compilation-minor-mode.
@@ -2108,6 +2109,54 @@ diff-goto-source
       (goto-char (+ (car pos) (cdr src)))
       (when buffer (next-error-found buffer (current-buffer))))))
 
+(defun diff-kill-ring-save (beg end &optional reverse)
+  "Save contents of the region between BEG and END akin to `kill-ring-save'.
+By default the command will copy the text that applying the diff would
+produce, along with the text between hunks.  If REVERSE is non-nil, or
+the command was invoked with a prefix argument, copy the deleted text."
+  (interactive
+   (append (if (use-region-p)
+               (list (region-beginning) (region-end))
+             (save-excursion
+               (list (diff-beginning-of-hunk)
+                     (diff-end-of-hunk))))
+           (list current-prefix-arg)))
+  (unless (derived-mode-p 'diff-mode)
+    (user-error "Command can only be invoked in a diff-buffer"))
+  (let ((parts '()))
+    (save-excursion
+      (goto-char beg)
+      (catch 'break
+        (while t
+          (let ((hunk (diff-hunk-text
+                       (buffer-substring
+                        (save-excursion (diff-beginning-of-hunk))
+                        (save-excursion (min (diff-end-of-hunk) end)))
+                       (not reverse)
+                       (save-excursion
+                         (- (point) (diff-beginning-of-hunk))))))
+            (push (substring (car hunk) (cdr hunk))
+                  parts))
+          ;; check if we have copied everything
+          (diff-end-of-hunk)
+          (when (<= end (point)) (throw 'break t))
+          ;; copy the text between hunks
+          (let ((inhibit-message t) start)
+            (save-window-excursion
+              (save-excursion
+                (forward-line -1)
+                ;; FIXME: Detect if the line we jump to doesn't match
+                ;; the line in the diff.
+                (diff-goto-source t)
+                (forward-line +1)
+                (setq start (point))))
+            (save-window-excursion
+              (diff-goto-source t)
+              (push (buffer-substring start (point))
+                    parts))))))
+    (kill-new (string-join (nreverse parts)))
+    (setq deactivate-mark t)
+    (message (if reverse "Copied original text" "Copied modified text"))))
 
 (defun diff-current-defun ()
   "Find the name of function at point.
-- 
2.46.0

-- 
	Philip Kaludercic on peregrine