1 files changed, 111 insertions, 59 deletions

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 5c0fb5fba4c..559310ff770 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -201,8 +201,9 @@ The default \"-b\" means to ignore whitespace-only changes,
 (defvar-keymap diff-mode-shared-map
   :doc "Bindings for read-only `diff-mode' buffers.
 These bindings are also available with an ESC prefix
-(i.e. a \\=`M-' prefix) in read-write `diff-mode' buffers,
-and with a `diff-minor-mode-prefix' prefix in `diff-minor-mode'.
+(i.e. a \\=`M-' prefix) in all `diff-mode' buffers, including in
+particular read-write `diff-mode' buffers, and with a
+`diff-minor-mode-prefix' prefix in `diff-minor-mode'.
 See also `diff-mode-read-only-map'."
   "n" #'diff-hunk-next
   "N" #'diff-file-next
@@ -217,14 +218,7 @@ See also `diff-mode-read-only-map'."
   "RET" #'diff-goto-source
   "<mouse-2>" #'diff-goto-source
   "o" #'diff-goto-source                ; other-window
-  "<remap> <undo>" #'undo-ignore-read-only
-
-  ;; The foregoing commands don't affect buffers beyond this one.
-  ;; The following command is the only one that has a single-letter
-  ;; binding and which affects buffers beyond this one.
-  ;; However, the following command asks for confirmation by default,
-  ;; so that seems okay.  --spwhitton
-  "u" #'diff-revert-and-kill-hunk)
+  "<remap> <undo>" #'undo-ignore-read-only)
 
 ;; Not `diff-read-only-mode-map' because there is no such mode
 ;; `diff-read-only-mode'; see comment above.
@@ -233,15 +227,28 @@ See also `diff-mode-read-only-map'."
   :doc "Additional bindings for read-only `diff-mode' buffers.
 Most of the bindings for read-only `diff-mode' buffers are in
 `diff-mode-shared-map'.  This map contains additional bindings for
-read-only `diff-mode' buffers that are *not* available with an ESC
-prefix (i.e. a \\=`M-' prefix) in read-write `diff-mode' buffers."
+read-only `diff-mode' buffers that are *not* also available with an ESC
+prefix (i.e. a \\=`M-' prefix) in read-write (nor read-only) `diff-mode'
+buffers."
   ;; We don't want the following in read-write `diff-mode' buffers
   ;; because they hide useful `M-<foo>' global bindings when editing.
   "W" #'widen
   "w" #'diff-kill-ring-save
   "A" #'diff-ediff-patch
   "r" #'diff-restrict-view
-  "R" #'diff-reverse-direction)
+  "R" #'diff-reverse-direction
+  "s" #'diff-split-hunk
+
+  ;; The foregoing commands in `diff-mode-shared-map' and
+  ;; `diff-mode-read-only-map' don't affect buffers beyond this one.
+  ;; The following command is the only one that has a single-character
+  ;; binding and which affects buffers beyond this one.  However, the
+  ;; following command asks for confirmation by default, so that seems
+  ;; okay.  --spwhitton
+  "u" #'diff-revert-and-kill-hunk
+  ;; `diff-revert-and-kill-hunk' is the `diff-mode' analogue of what '@'
+  ;; does in VC-Dir, so give it the same short binding.
+  "@" #'diff-revert-and-kill-hunk)
 
 (defvar-keymap diff-mode-map
   :doc "Keymap for `diff-mode'.  See also `diff-mode-shared-map'."
@@ -882,31 +889,19 @@ If the prefix ARG is given, restrict the view to the current file instead."
     (goto-char (point-min))
     (re-search-forward diff-hunk-header-re nil t)))
 
-(defun diff-hunk-kill ()
-  "Kill the hunk at point."
-  (interactive)
-  (if (not (diff--some-hunks-p))
-      (error "No hunks")
-    (diff-beginning-of-hunk t)
-    (let* ((hunk-bounds (diff-bounds-of-hunk))
-           (file-bounds (ignore-errors (diff-bounds-of-file)))
-           ;; If the current hunk is the only one for its file, kill the
-           ;; file header too.
-           (bounds (if (and file-bounds
-                            (progn (goto-char (car file-bounds))
-                                   (= (progn (diff-hunk-next) (point))
-                                      (car hunk-bounds)))
-                            (progn (goto-char (cadr hunk-bounds))
-                                   ;; bzr puts a newline after the last hunk.
-                                   (while (looking-at "^\n")
-                                     (forward-char 1))
-                                   (= (point) (cadr file-bounds))))
-                       file-bounds
-                     hunk-bounds))
-           (inhibit-read-only t))
-      (apply #'kill-region bounds)
-      (goto-char (car bounds))
-      (ignore-errors (diff-beginning-of-hunk t)))))
+(defun diff-hunk-kill (&optional beg end)
+  "Kill the hunk at point.
+When killing the last hunk left for a file, kill the file header too.
+Interactively, if the region is active, kill all hunks that the region
+overlaps.
+
+When called from Lisp with optional arguments BEG and END non-nil, kill
+all hunks overlapped by the region from BEG to END as though called
+interactively with an active region delimited by BEG and END."
+  (interactive "R")
+  (when (xor beg end)
+    (error "Invalid call to `diff-hunk-kill'"))
+  (diff--revert-kill-hunks beg end nil))
 
 ;; This is not `diff-kill-other-hunks' because we might need to make
 ;; copies of file headers in order to ensure the new kill ring entry
@@ -2282,6 +2277,83 @@ With a prefix argument, try to REVERSE the hunk."
   :type 'boolean
   :version "31.1")
 
+(defun diff--revert-kill-hunks (beg end revertp)
+  "Workhorse routine for killing hunks, after possibly reverting them.
+If BEG and END are nil, kill the hunk at point.
+Otherwise kill all hunks overlapped by region delimited by BEG and END.
+When killing a hunk that's the only one remaining for its file, kill the
+file header too.
+If REVERTP is non-nil, reverse-apply hunks before killing them."
+  ;; With BEG and END non-nil, we push each hunk to the kill ring
+  ;; separately.  If we want to push to the kill ring just once, we have
+  ;; to decide how to handle file headers such that the meanings of the
+  ;; hunks in the kill ring entry, considered as a whole patch, do not
+  ;; deviate too far from the meanings the hunks had in this buffer.
+  ;;
+  ;; For example, if we have a single hunk for one file followed by
+  ;; multiple hunks for another file, and we naïvely kill the single
+  ;; hunk and the first of the multiple hunks, our kill ring entry will
+  ;; be a patch applying those two hunks to the first file.  This is
+  ;; because killing the single hunk will have brought its file header
+  ;; with it, but not so killing the second hunk.  So we will have put
+  ;; together hunks that were previously for two different files.
+  ;;
+  ;; One option is to *copy* every file header that the region overlaps
+  ;; (and that we will not kill, because we are leaving other hunks for
+  ;; that file behind).  But then the text this command pushes to the
+  ;; kill ring would be different from the text it removes from the
+  ;; buffer, which would be unintuitive for an Emacs kill command.
+  ;;
+  ;; An alternative might be to have restrictions as follows:
+  ;;
+  ;;   Interactively, if the region is active, try to kill all hunks that the
+  ;;   region overlaps.  This works when either
+  ;;   - all the hunks the region overlaps are for the same file; or
+  ;;   - the last hunk the region overlaps is the last hunk for its file.
+  ;;   These restrictions are so that the text added to the kill ring does not
+  ;;   merge together hunks for different files under a single file header.
+  ;;
+  ;; We would error out if neither property is met.  When either holds,
+  ;; any file headers the region overlaps are ones we should kill.
+  (unless (diff--some-hunks-p)
+    (error "No hunks"))
+  (if beg
+      (save-excursion
+        (goto-char beg)
+        (setq beg (car (diff-bounds-of-hunk)))
+        (goto-char end)
+        (unless (looking-at diff-hunk-header-re)
+          (setq end (cadr (diff-bounds-of-hunk)))))
+    (pcase-setq `(,beg ,end) (diff-bounds-of-hunk)))
+  (when (or (not revertp) (null (diff-apply-buffer beg end t)))
+    (goto-char end)
+    (when-let* ((pos (diff--at-diff-header-p)))
+      (goto-char pos))
+    (setq beg (copy-marker beg) end (point-marker))
+    (unwind-protect
+        (cl-loop initially (goto-char beg)
+                 with inhibit-read-only = t
+                 for (hunk-beg hunk-end) = (diff-bounds-of-hunk)
+                 for file-bounds = (ignore-errors (diff-bounds-of-file))
+                 for (file-beg file-end) = file-bounds
+                 if (and file-bounds
+                         (progn
+                           (goto-char file-beg)
+                           (diff-hunk-next)
+                           (eq (point) hunk-beg))
+                         (progn
+                           (goto-char hunk-end)
+                           ;; bzr puts a newline after the last hunk.
+                           (while (looking-at "^\n") (forward-char 1))
+                           (eq (point) file-end)))
+                 do (kill-region file-beg file-end) (goto-char file-beg)
+                 else do (kill-region hunk-beg hunk-end) (goto-char hunk-beg)
+                 do (ignore-errors (diff-beginning-of-hunk t))
+                 until (or (< (point) (marker-position beg))
+                           (eql (point) (marker-position end))))
+      (set-marker beg nil)
+      (set-marker end nil))))
+
 (defun diff-revert-and-kill-hunk (&optional beg end)
   "Reverse-apply and then kill the hunk at point.  Save changed buffer.
 Interactively, if the region is active, reverse-apply and kill all
@@ -2307,27 +2379,7 @@ BEG and END."
     (error "Invalid call to `diff-revert-and-kill-hunk'"))
   (when (or (not diff-ask-before-revert-and-kill-hunk)
             (y-or-n-p "Really reverse-apply and kill hunk(s)?"))
-    (if beg
-        (save-excursion
-          (goto-char beg)
-          (setq beg (car (diff-bounds-of-hunk)))
-          (goto-char end)
-          (unless (looking-at diff-hunk-header-re)
-            (setq end (cadr (diff-bounds-of-hunk)))))
-      (pcase-setq `(,beg ,end) (diff-bounds-of-hunk)))
-    (when (null (diff-apply-buffer beg end t))
-      ;; Use `diff-hunk-kill' because it properly handles file headers.
-      (goto-char end)
-      (when-let* ((pos (diff--at-diff-header-p)))
-        (goto-char pos))
-      (setq beg (copy-marker beg) end (point-marker))
-      (unwind-protect
-          (cl-loop initially (goto-char beg)
-                   do (diff-hunk-kill)
-                   until (or (< (point) (marker-position beg))
-                             (eql (point) (marker-position end))))
-        (set-marker beg nil)
-        (set-marker end nil)))))
+    (diff--revert-kill-hunks beg end t)))
 
 (defun diff-apply-buffer (&optional beg end reverse test-or-no-save)
   "Apply the diff in the entire diff buffer.