GNU bug report logs - #66113
Apply the entire diff buffer

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 66113 in the body.
You can then email your comments to 66113 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Juri Linkov <juri <at> linkov.net>
To: bug-gnu-emacs <at> gnu.org
Subject: Apply the entire diff buffer
Date: Wed, 20 Sep 2023 09:46:18 +0300

[Message part 1 (text/plain, inline)]

After discussing on emacs-devel, here is a complete patch for the feature
of applying the entire diff buffer:

[diff-apply-buffer.patch (text/x-diff, inline)]

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index d776375d681..4633d5896a7 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-m a" #'diff-apply-buffer
   "C-c C-e" #'diff-ediff-patch
   "C-c C-n" #'diff-restrict-view
   "C-c C-s" #'diff-split-hunk
@@ -2054,6 +2055,40 @@ diff-kill-applied-hunks
           (diff-hunk-kill)
         (diff-hunk-next)))))
 
+(defun diff-apply-buffer ()
+  "Apply the diff in the entire diff buffer.
+When applying all hunks was successful, then save the changed buffers."
+  (interactive)
+  (let ((buffers nil)
+        (failures 0)
+        (diff-refine nil))
+    (save-excursion
+      (goto-char (point-min))
+      (diff-beginning-of-hunk t)
+      (while (pcase-let ((`(,buf ,line-offset ,pos ,_src ,dst ,switched)
+                          (diff-find-source-location nil nil)))
+               (cond ((and line-offset (not switched))
+                      (with-current-buffer buf
+                        (goto-char (car pos))
+                        (delete-region (car pos) (cdr pos))
+                        (insert (car dst))
+                        (when buffer-file-name
+                          (push (current-buffer) buffers))))
+                     (t (setq failures (1+ failures))))
+               (not (or (eq (prog1 (point) (diff-hunk-next)) (point))
+                        (eobp))))))
+    (setq buffers (delete-dups buffers))
+    (cond ((zerop failures)
+           (dolist (buf (reverse buffers))
+             (with-current-buffer buf
+               (save-buffer)))
+           (message "Saved %d buffers" (length buffers)))
+          (t
+           (dolist (buf buffers)
+             (with-current-buffer buf
+               (display-buffer buf)))
+           (message "%d hunks failed; no buffers saved" failures)))))
+
 (defalias 'diff-mouse-goto-source #'diff-goto-source)
 
 (defun diff-goto-source (&optional other-file event)

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>, 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Fri, 22 Sep 2023 04:38:25 +0300

Hi Juri,

On 20/09/2023 09:46, Juri Linkov wrote:
> +          (t
> +           (dolist (buf buffers)
> +             (with-current-buffer buf
> +               (display-buffer buf)))
> +           (message "%d hunks failed; no buffers saved" failures)))))

What happens next in this case? How do you undo in the buffers that had 
the patch hunks already applied?

Any change you wanted to work on the idea of the "atomic rollback" as well?

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Fri, 22 Sep 2023 09:45:32 +0300

>> +          (t
>> +           (dolist (buf buffers)
>> +             (with-current-buffer buf
>> +               (display-buffer buf)))
>> +           (message "%d hunks failed; no buffers saved" failures)))))
>
> What happens next in this case? How do you undo in the buffers that had the
> patch hunks already applied?

Manually, like in case of ediff-patch-buffer.

> Any change you wanted to work on the idea of the "atomic rollback" as well?

This would be an unreliable feature: in case of diff-apply creates a mess,
such automatic undo can create more mess, because there are many different
strategies to undo the mess such as using undo-auto-amalgamate, or
applying the reverse diff partially, doing more damage in case when
buffers were already modified before diff-apply.

But fortunately the need to undo will be extremely rare, because
when patch hunks are already applied, it reports the failure,
but doesn't modify the source buffer.  Therefore there is
nothing to undo!

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Fri, 22 Sep 2023 16:49:29 +0300

On 22/09/2023 09:45, Juri Linkov wrote:
>>> +          (t
>>> +           (dolist (buf buffers)
>>> +             (with-current-buffer buf
>>> +               (display-buffer buf)))
>>> +           (message "%d hunks failed; no buffers saved" failures)))))
>>
>> What happens next in this case? How do you undo in the buffers that had the
>> patch hunks already applied?
> 
> Manually, like in case of ediff-patch-buffer.

I've never used this, so I don't understand.

>> Any change you wanted to work on the idea of the "atomic rollback" as well?
> 
> This would be an unreliable feature: in case of diff-apply creates a mess,
> such automatic undo can create more mess, because there are many different
> strategies to undo the mess such as using undo-auto-amalgamate, or
> applying the reverse diff partially, doing more damage in case when
> buffers were already modified before diff-apply.

How about we save the tips of buffer-undo-list, then in case the buffer 
needs reverting, basically 'undo' each of the buffers until the saved 
"tip" is reached? I don't have a quick code anippet, but that seems doable.

> But fortunately the need to undo will be extremely rare, because
> when patch hunks are already applied, it reports the failure,
> but doesn't modify the source buffer.  Therefore there is
> nothing to undo!

The problem situations is, of course, when one of the hunks (somewhere 
in the middle or near the end) fails to apply cleanly or at all.

Another approach would be to first go through the patch and check that 
all hunks apply without problems, and then, on the second pass, actually 
apply them.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Fri, 22 Sep 2023 18:48:49 +0300

>>>> +          (t
>>>> +           (dolist (buf buffers)
>>>> +             (with-current-buffer buf
>>>> +               (display-buffer buf)))
>>>> +           (message "%d hunks failed; no buffers saved" failures)))))
>>>
>>> What happens next in this case? How do you undo in the buffers that had the
>>> patch hunks already applied?
>> Manually, like in case of ediff-patch-buffer.
>
> I've never used this, so I don't understand.

ediff-patch-buffer is an example when trying to roll back a mess
creates more mess with .orig/.rej files.

>>> Any change you wanted to work on the idea of the "atomic rollback" as well?
>> This would be an unreliable feature: in case of diff-apply creates
>> a mess,
>> such automatic undo can create more mess, because there are many different
>> strategies to undo the mess such as using undo-auto-amalgamate, or
>> applying the reverse diff partially, doing more damage in case when
>> buffers were already modified before diff-apply.
>
> How about we save the tips of buffer-undo-list, then in case the buffer
> needs reverting, basically 'undo' each of the buffers until the saved "tip"
> is reached? I don't have a quick code anippet, but that seems doable.

Maybe, but I definitely won't use such an option.

>> But fortunately the need to undo will be extremely rare, because
>> when patch hunks are already applied, it reports the failure,
>> but doesn't modify the source buffer.  Therefore there is
>> nothing to undo!
>
> The problem situations is, of course, when one of the hunks (somewhere in
> the middle or near the end) fails to apply cleanly or at all.

Indeed, this is a problematic case when one of the hunks fails
whereas all other hunks are applied cleanly.  This sometimes happens
when receiving an external patch, but the source already changed.

> Another approach would be to first go through the patch and check that all
> hunks apply without problems, and then, on the second pass, actually apply
> them.

This is a better option, thanks for the idea, will try to do this with
'diff-test-hunk' in a loop.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Sat, 23 Sep 2023 20:52:27 +0300

[Message part 1 (text/plain, inline)]

>> Another approach would be to first go through the patch and check that all
>> hunks apply without problems, and then, on the second pass, actually apply
>> them.
>
> This is a better option, thanks for the idea, will try to do this with
> 'diff-test-hunk' in a loop.

Indeed, this is much better:

[diff-apply-buffer.patch (text/x-diff, inline)]

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index d776375d681..2b0daabb12c 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-m a" #'diff-apply-buffer
   "C-c C-e" #'diff-ediff-patch
   "C-c C-n" #'diff-restrict-view
   "C-c C-s" #'diff-split-hunk
@@ -2054,6 +2055,39 @@ diff-kill-applied-hunks
           (diff-hunk-kill)
         (diff-hunk-next)))))
 
+(defun diff-apply-buffer ()
+  "Apply the diff in the entire diff buffer.
+When applying all hunks was successful, then save the changed buffers."
+  (interactive)
+  (let ((buffer-edits nil)
+        (failures 0)
+        (diff-refine nil))
+    (save-excursion
+      (goto-char (point-min))
+      (diff-beginning-of-hunk t)
+      (while (pcase-let ((`(,buf ,line-offset ,pos ,_src ,dst ,switched)
+                          (diff-find-source-location nil nil)))
+               (cond ((and line-offset (not switched))
+                      (push (cons pos dst)
+                            (alist-get buf buffer-edits)))
+                     (t (setq failures (1+ failures))))
+               (not (or (eq (prog1 (point) (diff-hunk-next)) (point))
+                        (eobp))))))
+    (cond ((zerop failures)
+           (dolist (buf-edits (reverse buffer-edits))
+             (with-current-buffer (car buf-edits)
+               (dolist (edit (cdr buf-edits))
+                 (let ((pos (car edit))
+                       (dst (cdr edit))
+                       (inhibit-read-only t))
+                   (goto-char (car pos))
+                   (delete-region (car pos) (cdr pos))
+                   (insert (car dst))))
+               (save-buffer)))
+           (message "Saved %d buffers" (length buffer-edits)))
+          (t
+           (message "%d hunks failed; no buffers changed" failures)))))
+
 (defalias 'diff-mouse-goto-source #'diff-goto-source)
 
 (defun diff-goto-source (&optional other-file event)

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Sun, 24 Sep 2023 04:34:33 +0300

On 23/09/2023 20:52, Juri Linkov wrote:
> +(defun diff-apply-buffer ()
> +  "Apply the diff in the entire diff buffer.
> +When applying all hunks was successful, then save the changed buffers."
> +  (interactive)
> +  (let ((buffer-edits nil)
> +        (failures 0)
> +        (diff-refine nil))
> +    (save-excursion
> +      (goto-char (point-min))
> +      (diff-beginning-of-hunk t)
> +      (while (pcase-let ((`(,buf ,line-offset ,pos ,_src ,dst ,switched)
> +                          (diff-find-source-location nil nil)))
> +               (cond ((and line-offset (not switched))
> +                      (push (cons pos dst)
> +                            (alist-get buf buffer-edits)))
> +                     (t (setq failures (1+ failures))))
> +               (not (or (eq (prog1 (point) (diff-hunk-next)) (point))
> +                        (eobp))))))
> +    (cond ((zerop failures)
> +           (dolist (buf-edits (reverse buffer-edits))
> +             (with-current-buffer (car buf-edits)
> +               (dolist (edit (cdr buf-edits))
> +                 (let ((pos (car edit))
> +                       (dst (cdr edit))
> +                       (inhibit-read-only t))
> +                   (goto-char (car pos))
> +                   (delete-region (car pos) (cdr pos))
> +                   (insert (car dst))))
> +               (save-buffer)))
> +           (message "Saved %d buffers" (length buffer-edits)))
> +          (t
> +           (message "%d hunks failed; no buffers changed" failures)))))

Sorry, was there supposed to be a call to diff-test-hunk here? Or I'm 
just not following the implementation.

I tried testing it out too. There is a patch where the third hunk 
doesn't apply (errors with "can't find the text to patch" in regular 
usage). This command ends with cryptic "No next hunk".

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Sun, 24 Sep 2023 10:34:32 +0300

>> +(defun diff-apply-buffer ()
>> +  "Apply the diff in the entire diff buffer.
>> +When applying all hunks was successful, then save the changed buffers."
>> +  (interactive)
>> +  (let ((buffer-edits nil)
>> +        (failures 0)
>> +        (diff-refine nil))
>> +    (save-excursion
>> +      (goto-char (point-min))
>> +      (diff-beginning-of-hunk t)
>> +      (while (pcase-let ((`(,buf ,line-offset ,pos ,_src ,dst ,switched)
>> +                          (diff-find-source-location nil nil)))
>> +               (cond ((and line-offset (not switched))
>> +                      (push (cons pos dst)
>> +                            (alist-get buf buffer-edits)))
>> +                     (t (setq failures (1+ failures))))
>> +               (not (or (eq (prog1 (point) (diff-hunk-next)) (point))
>> +                        (eobp))))))
>> +    (cond ((zerop failures)
>> +           (dolist (buf-edits (reverse buffer-edits))
>> +             (with-current-buffer (car buf-edits)
>> +               (dolist (edit (cdr buf-edits))
>> +                 (let ((pos (car edit))
>> +                       (dst (cdr edit))
>> +                       (inhibit-read-only t))
>> +                   (goto-char (car pos))
>> +                   (delete-region (car pos) (cdr pos))
>> +                   (insert (car dst))))
>> +               (save-buffer)))
>> +           (message "Saved %d buffers" (length buffer-edits)))
>> +          (t
>> +           (message "%d hunks failed; no buffers changed" failures)))))
>
> Sorry, was there supposed to be a call to diff-test-hunk here? Or I'm just
> not following the implementation.

diff-test-hunk was just an example of implementation.  But the same way as
diff-apply-hunk is not used here, also only the logic of diff-test-hunk is used.

> I tried testing it out too. There is a patch where the third hunk doesn't
> apply (errors with "can't find the text to patch" in regular usage). This
> command ends with cryptic "No next hunk".

This is what I expected that diff-hunk-next might signal an error
that we should catch, but still couldn't find a patch that fails
with such error.  So I will try to construct such a patch manually.

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Sun, 24 Sep 2023 13:58:59 +0300

On 24/09/2023 10:34, Juri Linkov wrote:
>> Sorry, was there supposed to be a call to diff-test-hunk here? Or I'm just
>> not following the implementation.
> diff-test-hunk was just an example of implementation.  But the same way as
> diff-apply-hunk is not used here, also only the logic of diff-test-hunk is used.

Fair enough.

>> I tried testing it out too. There is a patch where the third hunk doesn't
>> apply (errors with "can't find the text to patch" in regular usage). This
>> command ends with cryptic "No next hunk".
> This is what I expected that diff-hunk-next might signal an error
> that we should catch, but still couldn't find a patch that fails
> with such error.  So I will try to construct such a patch manually.

Try the patch attached to this message: 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63896#5

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Mon, 25 Sep 2023 20:49:04 +0300

[Message part 1 (text/plain, inline)]

>>> I tried testing it out too. There is a patch where the third hunk doesn't
>>> apply (errors with "can't find the text to patch" in regular usage). This
>>> command ends with cryptic "No next hunk".
>> This is what I expected that diff-hunk-next might signal an error
>> that we should catch, but still couldn't find a patch that fails
>> with such error.  So I will try to construct such a patch manually.
>
> Try the patch attached to this message:
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63896#5

Thanks, I forgot about signatures in git patches.
diff-mode already handles them in diff-fixup-modifs:

                        ;; In git format-patch "^-- $" signifies
                        ;; the end of the patch.
			(and (eq diff-buffer-type 'git)
			     (looking-at "^-- $"))

So I just copied this code here:

[diff-apply-buffer.patch (text/x-diff, inline)]

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index d776375d681..8f70731c7dc 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-m a" #'diff-apply-buffer
   "C-c C-e" #'diff-ediff-patch
   "C-c C-n" #'diff-restrict-view
   "C-c C-s" #'diff-split-hunk
@@ -2054,6 +2055,43 @@ diff-kill-applied-hunks
           (diff-hunk-kill)
         (diff-hunk-next)))))
 
+(defun diff-apply-buffer ()
+  "Apply the diff in the entire diff buffer.
+When applying all hunks was successful, then save the changed buffers."
+  (interactive)
+  (let ((buffer-edits nil)
+        (failures 0)
+        (diff-refine nil))
+    (save-excursion
+      (goto-char (point-min))
+      (diff-beginning-of-hunk t)
+      (while (pcase-let ((`(,buf ,line-offset ,pos ,_src ,dst ,switched)
+                          (diff-find-source-location nil nil)))
+               (cond ((and line-offset (not switched))
+                      (push (cons pos dst)
+                            (alist-get buf buffer-edits)))
+                     (t (setq failures (1+ failures))))
+               (not (or (eq (prog1 (point) (diff-hunk-next)) (point))
+                        (eobp)
+                        ;; In git format-patch "^-- $" signifies
+                        ;; the end of the patch.
+                        (and (eq diff-buffer-type 'git)
+                             (looking-at-p "^-- $")))))))
+    (cond ((zerop failures)
+           (dolist (buf-edits (reverse buffer-edits))
+             (with-current-buffer (car buf-edits)
+               (dolist (edit (cdr buf-edits))
+                 (let ((pos (car edit))
+                       (dst (cdr edit))
+                       (inhibit-read-only t))
+                   (goto-char (car pos))
+                   (delete-region (car pos) (cdr pos))
+                   (insert (car dst))))
+               (save-buffer)))
+           (message "Saved %d buffers" (length buffer-edits)))
+          (t
+           (message "%d hunks failed; no buffers changed" failures)))))
+
 (defalias 'diff-mouse-goto-source #'diff-goto-source)
 
 (defun diff-goto-source (&optional other-file event)

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

From: Dmitry Gutov <dmitry <at> gutov.dev>
To: Juri Linkov <juri <at> linkov.net>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Tue, 26 Sep 2023 02:07:33 +0300

On 25/09/2023 20:49, Juri Linkov wrote:
>>>> I tried testing it out too. There is a patch where the third hunk doesn't
>>>> apply (errors with "can't find the text to patch" in regular usage). This
>>>> command ends with cryptic "No next hunk".
>>> This is what I expected that diff-hunk-next might signal an error
>>> that we should catch, but still couldn't find a patch that fails
>>> with such error.  So I will try to construct such a patch manually.
>> Try the patch attached to this message:
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=63896#5
> Thanks, I forgot about signatures in git patches.
> diff-mode already handles them in diff-fixup-modifs:
> 
>                          ;; In git format-patch "^-- $" signifies
>                          ;; the end of the patch.
> 			(and (eq diff-buffer-type 'git)
> 			     (looking-at "^-- $"))
> 
> So I just copied this code here:

That works. Thanks!

Ideally, I think point would move to the first failing hunk. But that's 
not urgent, could be a TODO for later.

Aside from that, I'm still not crazy about the binding. But that's not a 
technical issue.

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

From: Juri Linkov <juri <at> linkov.net>
To: Dmitry Gutov <dmitry <at> gutov.dev>
Cc: 66113 <at> debbugs.gnu.org
Subject: Re: bug#66113: Apply the entire diff buffer
Date: Wed, 27 Sep 2023 20:36:49 +0300

close 66113 30.0.50
thanks

>>                          ;; In git format-patch "^-- $" signifies
>>                          ;; the end of the patch.
>> 			(and (eq diff-buffer-type 'git)
>> 			     (looking-at "^-- $"))
>> So I just copied this code here:
>
> That works. Thanks!

Actually this doesn't work for bzr that has a bug that adds
an extra line at the end.  So I pushed a better fix.

> Ideally, I think point would move to the first failing hunk. But that's not
> urgent, could be a TODO for later.

There are many variants what would be better to do here.
Maybe also to show all failing hunks.  And leaving point
at the beginning also makes sense, because then the user
could start applying hunks one by one with 'C-c C-a'
from the beginning until encountering the failing hunk
and handle it manually.

> Aside from that, I'm still not crazy about the binding. But that's not
> a technical issue.

Agreed, the key binding is the best among bad variants.

Previous Next

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