bug#73407: [PATCH v4] bug#73407: 31.0.50; Add diff-discard-hunk

[Sean Whitton <spwhitton@spwhitton.name>]

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

Hello,

On Tue 24 Sep 2024 at 03:21pm +03, Eli Zaretskii wrote:

> We should come up with a better name for this command.
> diff-discard-hunk tells me the command will discard the hunk, but says
> nothing about applying it, let alone reverse-applying it.  How about
> diff-revert-hunk, or maybe diff-revert-and-discard?

Oh, nice.  I was just following Magit, but including "revert" is better,
indeed.  I think I prefer diff-revert-and-kill-hunk.

>> +This command is useful in buffers generated by @w{@kbd{C-x v =}} and
>> +@w{@kbd{C-x v D}} (@pxref{Old Revisions}).  You can use this command to
>> +remove hunks you never intend to commit, such as temporary debug prints,
>> +and the like.
>
> This text could also use some clarification: "remove hunks you never
> intended to commit" only hints on what it actually does.  A better
> description would be something like "undo some of the changes you made
> that you didn't intend".  I wouldn't mention "commit" at all, since
> AFAIU this command is not specific to VC and doesn't require a VCS.

Yes, this is the diff-mode manual, so that makes sense.
I've made a similar change in the docstring but kept a reference to
committing, there.

>> -           (message "%d hunks failed; no buffers changed" failures)))))
>> +           (message "%d hunks failed; no buffers changed" failures)
>> +           failures))))
>
> This comes from existing text, but still: what does the above say when
> there's only 1 failure? does it say "1 hunks failed"?  If so, can we
> improve the handling of singular/plural here?

I've installed a fix for that.

On Tue 24 Sep 2024 at 03:22pm +03, Dmitry Gutov wrote:

> I'm a little concerned about the binding.
>
> We have 'diff-hunk-kill' on M-k (and also 'k'), and this new addition is
> actually more destructive. I'm expecting users will trip over the difference
> and call one of them when they wanted to call the other.

Yeah, and thinking about it more, it also doesn't really match what C-c
C-k does elsewhere in Emacs.  I've made it C-c M-r in the attached v4.

-- 
Sean Whitton

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v4-0001-New-command-diff-revert-and-kill-hunk.patch --]
[-- Type: text/x-patch, Size: 6207 bytes --]

From fc5e3a5be18f9b1532490c4c6b81b90b83c0acf6 Mon Sep 17 00:00:00 2001
From: Sean Whitton <spwhitton@spwhitton.name>
Date: Tue, 24 Sep 2024 09:38:43 +0100
Subject: [PATCH v4] New command diff-revert-and-kill-hunk

* lisp/vc/diff-mode.el (diff-revert-and-kill-hunk): New
command (bug#73407).
(diff-apply-buffer): New optional BEG and END arguments.  Return
nil if buffers were saved, or the number of failed applications.
(diff-mode-map): Bind diff-revert-and-kill-hunk to C-c M-r.
(diff-mode-menu): New entry "Revert and kill hunk".
* doc/emacs/files.texi (Diff Mode):
* etc/NEWS: Document the new command.
---
 doc/emacs/files.texi | 11 +++++++++++
 etc/NEWS             |  5 +++++
 lisp/vc/diff-mode.el | 41 ++++++++++++++++++++++++++++++++++++-----
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/doc/emacs/files.texi b/doc/emacs/files.texi
index 709cb0910e6..434fb30484a 100644
--- a/doc/emacs/files.texi
+++ b/doc/emacs/files.texi
@@ -1682,6 +1682,17 @@ Diff Mode
 version.  If @code{diff-jump-to-old-file} is non-@code{nil}, apply the
 hunk to the ``old'' version of the file instead.
 
+@findex diff-revert-and-kill-hunk
+@item C-c M-r
+Reverse-apply this hunk to the target file, and then kill it
+(@code{diff-revert-and-kill-hunk}).  Save the buffer visiting the target
+file.
+
+This command is useful in buffers generated by @w{@kbd{C-x v =}} and
+@w{@kbd{C-x v D}} (@pxref{Old Revisions}).  These buffers present you
+with a view of the changes you've made, and then you can use this
+command to drop changes you didn't intend, or no longer want.
+
 @findex diff-apply-buffer
 @item C-c @key{RET} a
 Apply all the hunks in the buffer (@code{diff-apply-buffer}).  If the
diff --git a/etc/NEWS b/etc/NEWS
index c1a0524c8ba..7b85b9cbc04 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -341,6 +341,11 @@ according to diffs in the current buffer, but without applying the diffs
 to the original text.  If the selected range extends a hunk, the
 command attempts to look up and copy the text in-between the hunks.
 
++++
+*** New command 'diff-revert-and-kill-hunk' bound to C-c M-r.
+This command reverse-applies the hunk at point, and then kills it.
+This is useful in buffers generated by C-x v = and C-x v D.
+
 ** php-ts-mode
 
 ---
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index be3d94db011..9e39e58e69b 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -218,6 +218,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 M-r" #'diff-revert-and-kill-hunk
   "C-c C-m a" #'diff-apply-buffer
   "C-c C-e" #'diff-ediff-patch
   "C-c C-n" #'diff-restrict-view
@@ -242,6 +243,8 @@ diff-mode-menu
      :help "Apply the current hunk to the source file and go to the next"]
     ["Test applying hunk"	diff-test-hunk
      :help "See whether it's possible to apply the current hunk"]
+    ["Revert and kill hunk"     diff-revert-and-kill-hunk
+     :help "Reverse-apply and then kill the current hunk."]
     ["Apply all hunks"		diff-apply-buffer
      :help "Apply all hunks in the current diff buffer"]
     ["Apply diff with Ediff"	diff-ediff-patch
@@ -2050,15 +2053,40 @@ diff-kill-applied-hunks
           (diff-hunk-kill)
         (diff-hunk-next)))))
 
-(defun diff-apply-buffer ()
+(defun diff-revert-and-kill-hunk ()
+  "Reverse-apply and then kill the hunk at point.  Save changed buffer.
+
+This command is useful in buffers generated by \\[vc-diff] and \\[vc-root-diff],
+especially when preparing to commit the patch with \\[vc-next-action].
+You can use \\<diff-mode-map>\\[diff-hunk-kill] to temporarily remove changes that you intend to
+include in a separate commit or commits, and you can use this command
+to permanently drop changes you didn't intend, or no longer want."
+  (interactive)
+  (when (yes-or-no-p "Really reverse-apply and kill this hunk?")
+    (cl-destructuring-bind (beg end) (diff-bounds-of-hunk)
+      (diff-reverse-direction beg end)
+      (condition-case ret
+          (diff-apply-buffer beg end)
+        ;; Reversing the hunk is an implementation detail, so ensure the
+        ;; user never sees it.
+        (error (diff-reverse-direction beg end)
+               (signal (car ret) (cdr ret)))
+        (:success (if ret
+                      (diff-reverse-direction beg end)
+                    (diff-hunk-kill)))))))
+
+(defun diff-apply-buffer (&optional beg end)
   "Apply the diff in the entire diff buffer.
-When applying all hunks was successful, then save the changed buffers."
+When applying all hunks was successful, then save the changed buffers.
+When called from Lisp with optional arguments, restrict the application
+to hunks lying between BEG and END.  Returns nil if buffers were saved,
+or the number of failed applications."
   (interactive)
   (let ((buffer-edits nil)
         (failures 0)
         (diff-refine nil))
     (save-excursion
-      (goto-char (point-min))
+      (goto-char (or beg (point-min)))
       (diff-beginning-of-hunk t)
       (while (pcase-let ((`(,buf ,line-offset ,pos ,_src ,dst ,switched)
                           (diff-find-source-location nil nil)))
@@ -2068,6 +2096,7 @@ diff-apply-buffer
                      (t (setq failures (1+ failures))))
                (and (not (eq (prog1 (point) (ignore-errors (diff-hunk-next)))
                              (point)))
+                    (or (not end) (< (point) end))
                     (looking-at-p diff-hunk-header-re)))))
     (cond ((zerop failures)
            (dolist (buf-edits (reverse buffer-edits))
@@ -2080,10 +2109,12 @@ diff-apply-buffer
                    (delete-region (car pos) (cdr pos))
                    (insert (car dst))))
                (save-buffer)))
-           (message "Saved %d buffers" (length buffer-edits)))
+           (message "Saved %d buffers" (length buffer-edits))
+           nil)
           (t
            (message "%d %s failed; no buffers changed"
-                    failures (if (> failures 1) "hunks" "hunk"))))))
+                    failures (if (> failures 1) "hunks" "hunk"))
+           failures))))
 
 (defalias 'diff-mouse-goto-source #'diff-goto-source)
 
-- 
2.39.5