bug#17544: 24.3; [PATCH] Improved diff-mode navigation/manipulation

[Dima Kogan]

Navigation and use of diff buffers had several annoying corner cases that this
patch fixes. These corner cases were largely due to inconsistent treatment of
file headers. Say you have a diff such as this:

 --- aaa
 +++ bbb
 @@ -52,7 +52,7 @@
 hunk1
 @@ -74,7 +74,7 @@
 hunk2
 --- ccc
 +++ ddd
 @@ -608,6 +608,6 @@
 hunk3
 @@ -654,7 +654,7 @@
 hunk4

The file headers here are the '---' and '+++' lines. With the point on such a
line, hunk operations would sometimes refer to the next hunk and sometimes to
the previous hunk. Most of the time it would be the previous hunk, which is not
what the user would expect. This patch consistently treats such headers as the
next hunk. So with this patch, if the point is on the '--- ccc' line, the point
is seen as referring to hunk3.

Specific behaviors this fixes are:

1. It should be possible to place the point in the middle of a diff buffer, and
press M-k repeatedly to kill hunks in the order they appear in the buffer. With
the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on
hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on
hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch.

2. Similarly, it should be possible to apply hunks in order. Previously with the
point at the start, C-c C-a would apply the hunk1, then move the point to the
first @@ header, and thus C-c C-a would try to apply the same hunk again.


>From 27d926293402c74eb3d83124e0287dd49cb4543a Mon Sep 17 00:00:00 2001
From: Dima Kogan <dima@secretsauce.net>
Date: Thu, 15 May 2014 00:24:00 -0700
Subject: [PATCH] Improved diff-mode navigation/manipulation

Navigation and use of diff buffers had several annoying corner cases that this
patch fixes. These corner cases were largely due to inconsistent treatment of
file headers. Say you have a diff such as this:

 --- aaa
 +++ bbb
 @@ -52,7 +52,7 @@
 hunk1
 @@ -74,7 +74,7 @@
 hunk2
 --- ccc
 +++ ddd
 @@ -608,6 +608,6 @@
 hunk3
 @@ -654,7 +654,7 @@
 hunk4

The file headers here are the '---' and '+++' lines. With the point on such a
line, hunk operations would sometimes refer to the next hunk and sometimes to
the previous hunk. Most of the time it would be the previous hunk, which is not
what the user would expect. This patch consistently treats such headers as the
next hunk. So with this patch, if the point is on the '--- ccc' line, the point
is seen as referring to hunk3.

Specific behaviors this fixes are:

1. It should be possible to place the point in the middle of a diff buffer, and
press M-k repeatedly to kill hunks in the order they appear in the buffer. With
the point on hunk1, M-k M-k would kill hunk1 then hunk2. With the point on
hunk3, it would kill hunk3 then hunk4; this is fine. However, with the point on
hunk2, it'd kill hunk2 then hunk1. This is fixed by this patch.

2. Similarly, it should be possible to apply hunks in order. Previously with the
point at the start, C-c C-a would apply the hunk1, then move the point to the
first @@ header, and thus C-c C-a would try to apply the same hunk again.
---
 lisp/vc/diff-mode.el | 77 ++++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 66 insertions(+), 11 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 923de9a..d334307 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -593,6 +593,58 @@ next hunk if TRY-HARDER is non-nil; otherwise signal an 
error."
 (easy-mmode-define-navigation
  diff-file diff-file-header-re "file" diff-end-of-file)
 
+(defun diff--wrap-hunk-navigation (isinteractive name orig count)
+  "I override the default diff-hunk-next/prev implementation by
+skipping any lines that are associated with this hunk but precede
+the hunk-start marker. For instance, a diff file could contain
+
+diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
+index 923de9a..6b1c24f 100644
+--- a/lisp/vc/diff-mode.el
++++ b/lisp/vc/diff-mode.el
+@@ -590,6 +590,22 @@
+.......
+
+If a point is on 'index', then the point is considered to be in
+this first hunk. Here I move the point to the @@... marker before
+executing the default diff-hunk-next/prev implementation to move
+to the NEXT marker"
+  (if (not isinteractive)
+      (funcall orig count)
+
+    (let ((start (point)))
+      (let ((hunk-bounds (diff-bounds-of-hunk)))
+        (goto-char (car hunk-bounds)))
+
+      (funcall orig count)
+
+      (when (not (looking-at diff-hunk-header-re))
+        (goto-char start)
+        (user-error (format "No %s hunk" name))))))
+
+(setq diff--hunk-prev-internal (symbol-function 'diff-hunk-prev))
+(defun diff-hunk-prev (&optional count)
+  "Go to the previous COUNT'th hunk"
+  (interactive "p")
+  (diff--wrap-hunk-navigation
+   (called-interactively-p 'interactive)
+   "prev"
+   diff--hunk-prev-internal
+   count))
+
+(setq diff--hunk-next-internal (symbol-function 'diff-hunk-next))
+(defun diff-hunk-next (&optional count)
+  "Go to the next COUNT'th hunk"
+  (interactive "p")
+  (diff--wrap-hunk-navigation
+   (called-interactively-p 'interactive)
+   "next"
+   diff--hunk-next-internal
+   count))
+
+
+
+
 (defun diff-bounds-of-hunk ()
   "Return the bounds of the diff hunk at point.
 The return value is a list (BEG END), which are the hunk's start
@@ -602,12 +654,13 @@ point is in a file header, return the bounds of the next 
hunk."
     (let ((pos (point))
          (beg (diff-beginning-of-hunk t))
          (end (diff-end-of-hunk)))
-      (cond ((>= end pos)
+      (cond ((> end pos)
             (list beg end))
            ;; If this hunk ends above POS, consider the next hunk.
            ((re-search-forward diff-hunk-header-re nil t)
             (list (match-beginning 0) (diff-end-of-hunk)))
-           (t (error "No hunk found"))))))
+            ;; There's no next hunk, so just take the one we have
+           (t (list beg end))))))
 
 (defun diff-bounds-of-file ()
   "Return the bounds of the file segment at point.
@@ -1683,8 +1736,9 @@ SRC and DST are the two variants of text as returned by 
`diff-hunk-text'.
 SWITCHED is non-nil if the patch is already applied.
 NOPROMPT, if non-nil, means not to prompt the user."
   (save-excursion
-    (let* ((other (diff-xor other-file diff-jump-to-old-file))
-          (char-offset (- (point) (diff-beginning-of-hunk t)))
+    (let* ((hunk-bounds (diff-bounds-of-hunk))
+           (other (diff-xor other-file diff-jump-to-old-file))
+           (char-offset (- (point) (goto-char (car hunk-bounds))))
            ;; Check that the hunk is well-formed.  Otherwise diff-mode and
            ;; the user may disagree on what constitutes the hunk
            ;; (e.g. because an empty line truncates the hunk mid-course),
@@ -1693,7 +1747,7 @@ NOPROMPT, if non-nil, means not to prompt the user."
           ;; Suppress check when NOPROMPT is non-nil (Bug#3033).
            (_ (unless noprompt (diff-sanity-check-hunk)))
           (hunk (buffer-substring
-                  (point) (save-excursion (diff-end-of-hunk) (point))))
+                  (point) (cadr hunk-bounds)))
           (old (diff-hunk-text hunk reverse char-offset))
           (new (diff-hunk-text hunk (not reverse) char-offset))
           ;; Find the location specification.
@@ -1802,7 +1856,7 @@ With a prefix argument, REVERSE the hunk."
       (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
       (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil)
       (when diff-advance-after-apply-hunk
-       (diff-hunk-next))))))
+       (call-interactively 'diff-hunk-next))))))
 
 
 (defun diff-test-hunk (&optional reverse)
@@ -1873,14 +1927,15 @@ For use in `add-log-current-defun-function'."
 (defun diff-ignore-whitespace-hunk ()
   "Re-diff the current hunk, ignoring whitespace differences."
   (interactive)
-  (let* ((char-offset (- (point) (diff-beginning-of-hunk t)))
+  (let* ((hunk-bounds (diff-bounds-of-hunk))
+         (char-offset (- (point) (goto-char (car hunk-bounds))))
         (opts (pcase (char-after) (?@ "-bu") (?* "-bc") (_ "-b")))
         (line-nb (and (or (looking-at "[^0-9]+\\([0-9]+\\)")
                           (error "Can't find line number"))
                       (string-to-number (match-string 1))))
         (inhibit-read-only t)
         (hunk (delete-and-extract-region
-               (point) (save-excursion (diff-end-of-hunk) (point))))
+                (point) (cadr hunk-bounds)))
         (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1.
         (file1 (make-temp-file "diff1"))
         (file2 (make-temp-file "diff2"))
@@ -1967,8 +2022,8 @@ For use in `add-log-current-defun-function'."
   (interactive)
   (require 'smerge-mode)
   (save-excursion
-    (diff-beginning-of-hunk t)
-    (let* ((start (point))
+    (let* ((hunk-bounds (diff-bounds-of-hunk))
+           (start (goto-char (car hunk-bounds)))
            (style (diff-hunk-style))    ;Skips the hunk header as well.
            (beg (point))
            (props-c '((diff-mode . fine) (face diff-refine-change)))
@@ -1976,7 +2031,7 @@ For use in `add-log-current-defun-function'."
            (props-a '((diff-mode . fine) (face diff-refine-added)))
            ;; Be careful to go back to `start' so diff-end-of-hunk gets
            ;; to read the hunk header's line info.
-           (end (progn (goto-char start) (diff-end-of-hunk) (point))))
+           (end (goto-char (cadr hunk-bounds))))
 
       (remove-overlays beg end 'diff-mode 'fine)
 
-- 
2.0.0.rc0