GNU bug report logs - #25400
M-p in diff-mode jumps too far

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 25400 in the body.
You can then email your comments to 25400 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: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: bug-gnu-emacs <at> gnu.org
Subject: M-p in diff-mode jumps too far
Date: Sun, 08 Jan 2017 16:21:16 -0500

Package: Emacs
Version: 26.0.50

In a buffer with more than one hunk, if I'm in the middle of hunk number
N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
number N-1 rather than to the header of hunk N.

This is contrary to the usual behavior of Emacs's navigation commands.


        Stefan



In GNU Emacs 26.0.50.1 (x86_64-unknown-linux-gnu, GTK+ Version 2.24.31)
 of 2017-01-06 built on ceviche
Repository revision: 074618e4af08e8d818c471f90c6de9b895980e05
Windowing system distributor 'The X.Org Foundation', version 11.0.11802000
System Description:	Debian GNU/Linux testing (stretch)

Recent messages:
~/src/elisp/tuareg-mode/tuareg.el and /home/monnier/src/emacs/elpa/packages/tuareg/tuareg.el are the same file
Hunk applied
Saving file /home/monnier/src/emacs/elpa/packages/tuareg/tuareg.el...
Wrote /home/monnier/src/emacs/elpa/packages/tuareg/tuareg.el
Invalid face reference: change-log-date-face [4 times]
Warning: turn-on-eldoc-mode is obsolete!
Warning: hide-sublevels is obsolete!
Invalid face reference: change-log-date-face [2 times]
Finding changes in /home/monnier/src/emacs/trunk/lisp/textmodes/rst.el...done
Invalid face reference: change-log-date-face [24 times]

Configured using:
 'configure -C --enable-checking --enable-check-lisp-object-type
 'CFLAGS=-Wall -g3 -Og -Wno-pointer-sign'
 PKG_CONFIG_PATH=/home/monnier/lib/pkgconfig'

Configured features:
XPM JPEG TIFF GIF PNG RSVG SOUND GPM DBUS GCONF GSETTINGS NOTIFY GNUTLS
LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS GTK2 X11

Important settings:
  value of $LANG: fr_CH.UTF-8
  locale-coding-system: utf-8-unix

Major mode: InactiveMinibuffer

Minor modes in effect:
  shell-dirtrack-mode: t
  diff-auto-refine-mode: t
  electric-pair-mode: t
  global-reveal-mode: t
  reveal-mode: t
  auto-insert-mode: t
  savehist-mode: t
  minibuffer-electric-default-mode: t
  global-compact-docstrings-mode: t
  url-handler-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  global-prettify-symbols-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  transient-mark-mode: t

Load-path shadows:
/home/monnier/src/emacs/elpa/packages/svg/svg hides /home/monnier/src/emacs/work/lisp/svg
/home/monnier/src/emacs/elpa/packages/ada-mode/ada-xref hides /home/monnier/src/emacs/work/lisp/progmodes/ada-xref
/home/monnier/src/emacs/elpa/packages/ada-mode/ada-mode hides /home/monnier/src/emacs/work/lisp/progmodes/ada-mode
/home/monnier/src/emacs/elpa/packages/ada-mode/ada-stmt hides /home/monnier/src/emacs/work/lisp/progmodes/ada-stmt
/home/monnier/src/emacs/elpa/packages/ada-mode/ada-prj hides /home/monnier/src/emacs/work/lisp/progmodes/ada-prj
/home/monnier/src/emacs/elpa/packages/landmark/landmark hides /home/monnier/src/emacs/work/lisp/obsolete/landmark
/home/monnier/src/emacs/elpa/packages/crisp/crisp hides /home/monnier/src/emacs/work/lisp/obsolete/crisp

Features:
(sort mail-extr emacsbug whitespace epa-file log-edit message sendmail
puny rfc822 mml mml-sec epa epg gnus-util rmail rmail-loaddefs mm-decode
mm-bodies mm-encode mail-parse rfc2231 rfc2047 rfc2045 mm-util
ietf-drums mail-prsvr mailabbrev mail-utils mailheader ffap vc-bzr
vc-src vc-sccs vc-svn vc-cvs vc-rcs vc-dir all hippie-exp ocamldebug
derived caml tuareg_indent tuareg caml-help caml-types caml-emacs
completion hibtypes hsys-www browse-url klink hib-kbd hib-debbugs
hib-social hsys-org org org-macro org-footnote org-pcomplete org-list
org-faces org-entities org-version ob-emacs-lisp ob ob-tangle ob-ref
ob-lob ob-table ob-exp org-src ob-keys ob-comint ob-core ob-eval
org-compat org-macs org-loaddefs hactypes hmail hargs hypb locate hbut
hact hpath hui-select hvar set hhist hbdata htz cal-julian cal-menu
calendar cal-loaddefs hbmap hmoccur hversion hload-path rect dabbrev
grep compile smerge-mode add-log log-view pcvs-util vc vc-dispatcher map
eieio-opt speedbar sb-image ezimage dframe help-fns radix-tree xscheme
warnings unsafep trace testcover shadow scheme re-builder profiler
inf-lisp ielm gmm-utils ert pp find-func ewoc debug elp edebug cl-indent
misearch multi-isearch css-mode smie autorevert filenotify doc-view
jka-compr image-mode dired dired-loaddefs executable copyright files-x
tramp-cache tramp-sh tramp tramp-compat tramp-loaddefs trampver
ucs-normalize shell pcomplete comint ansi-color ring parse-time
format-spec html5-schema cus-edit cus-start cus-load wid-edit vc-git
diff-mode filecache rng-xsd xsd-regexp rng-cmpct rng-nxml rng-valid
rng-loc rng-uri rng-parse nxml-parse rng-match rng-dt rng-util rng-pttrn
nxml-ns nxml-mode nxml-outln nxml-rap sgml-mode subr-x dom nxml-util
nxml-enc xmltok server time-date noutline outline easy-mmode flyspell
ispell checkdoc thingatpt load-dir elec-pair reveal autoinsert
proof-site proof-autoloads cl pg-vars savehist minibuf-eldef disp-table
compact-docstrings kotl-loaddefs advice info realgud-recursive-autoloads
finder-inf url-auth package epg-config url-handlers url-parse
auth-source eieio eieio-core cl-macs eieio-loaddefs password-cache
url-vars seq byte-opt gv bytecomp byte-compile cl-extra help-mode
easymenu cconv cl-loaddefs pcase cl-lib bbdb-autoloads mule-util tooltip
eldoc electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
term/x-win x-win term/common-win x-dnd tool-bar dnd fontset image
regexp-opt fringe tabulated-list replace newcomment text-mode elisp-mode
lisp-mode prog-mode register page menu-bar rfn-eshadow isearch timer
select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame cl-generic cham georgian utf-8-lang misc-lang
vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms cp51932
hebrew greek romanian slovak czech european ethiopic indian cyrillic
chinese composite charscript case-table epa-hook jka-cmpr-hook help
simple abbrev obarray minibuffer cl-preloaded nadvice loaddefs button
faces cus-face macroexp files text-properties overlay sha1 md5 base64
format env code-pages mule custom widget hashtable-print-readable
backquote dbusbind inotify dynamic-setting system-font-setting
font-render-setting move-toolbar gtk x-toolkit x multi-tty
make-network-process emacs)

Memory information:
((conses 8 616108 100562)
 (symbols 24 37145 0) (miscs 20 6978 1294) (strings 16 104997 17664)
 (string-bytes 1 2882737)
 (vectors 8 85773) (vector-slots 4 2368868 93788) (floats 8 1285 1070)
 (intervals 28 18695 3557)
 (buffers 520 64))

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

From: Mark Oteiza <mvoteiza <at> udel.edu>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: 25400 <at> debbugs.gnu.org
Subject: Re: bug#25400: M-p in diff-mode jumps too far
Date: Sun, 08 Jan 2017 18:27:44 -0500

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> Package: Emacs
> Version: 26.0.50
>
> In a buffer with more than one hunk, if I'm in the middle of hunk number
> N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
> number N-1 rather than to the header of hunk N.
>
> This is contrary to the usual behavior of Emacs's navigation commands.

Implemented in 2c8a7e5 (bug#17544). Apparently it was intended.

https://lists.gnu.org/archive/html/emacs-devel/2016-12/msg00222.html

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25105

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

From: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
To: Mark Oteiza <mvoteiza <at> udel.edu>
Cc: 25400 <at> debbugs.gnu.org
Subject: Re: bug#25400: M-p in diff-mode jumps too far
Date: Sun, 08 Jan 2017 23:34:52 -0500

>> In a buffer with more than one hunk, if I'm in the middle of hunk number
>> N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
>> number N-1 rather than to the header of hunk N.
>> 
>> This is contrary to the usual behavior of Emacs's navigation commands.

> Implemented in 2c8a7e5 (bug#17544). Apparently it was intended.

> https://lists.gnu.org/archive/html/emacs-devel/2016-12/msg00222.html

> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=25105

Maybe it's OK to change the behavior of `diff-hunk-prev` this way when
called from Elisp (I still find it odd, but it might work better for
the callers), but it's clearly not a good idea for M-p to behave
this way.

As pointed out elsewhere, it's particularly obnoxious from EOB (in which
case, you're not really "within N" but you're virtually on "the header
of the non-existent hunk N+1", so going to the header of N-1 is really
wrong).

I also dislike the fact that M-n doesn't let me get to EOB.


        Stefan

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Glenn Morris <rgm <at> gnu.org>, 25400 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net, tino.calancha <at> gmail.com,
 Mark Oteiza <mvoteiza <at> udel.edu>, Eli Zaretskii <eliz <at> gnu.org>,
 Dmitry Gutov <dgutov <at> yandex.ru>, 25105 <at> debbugs.gnu.org,
 Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25400: M-p in diff-mode jumps too far
Date: Wed, 11 Jan 2017 13:38:11 +0900

Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:

> In a buffer with more than one hunk, if I'm in the middle of hunk number
> N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
> number N-1 rather than to the header of hunk N.
>
> This is contrary to the usual behavior of Emacs's navigation commands.
>As pointed out elsewhere, it's particularly obnoxious from EOB (in which
>case, you're not really "within N" but you're virtually on "the header
>of the non-existent hunk N+1", so going to the header of N-1 is really
>wrong).
>
>I also dislike the fact that M-n doesn't let me get to EOB.

Following patch reverts commit 2c8a7e5.  Then it fixes dots 1. and 2.
described in the commit message of 2c8a7e5, i.e., Bug#17544.
This patch preserves the original definitions for 'diff-hunk-prev'
and 'diff-hunk-next'.

After applying locally this patch, you might want to do:
git diff 2c8a7e5^ HEAD lisp/vc/diff-mode.el
to see more clearly how it solves Bug#17544.

Regards,
Tino
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From a2dfaf390f4069b4e948dd0df84450359e7a0d92 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Wed, 11 Jan 2017 13:20:04 +0900
Subject: [PATCH] Fix Bugs 25105 and 25400

Revert 2c8a7e5 and implement a new fix for Bug#17544.
This patch satisfies 1. and 2. in 2c8a7e5 commit message.
The original definitions for 'diff-hunk-prev' and 'diff-hunk-next'
are preserved.
* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate.
(diff-beginning-of-hunk): Use it.
(diff-apply-hunk): Jump to beginning of hunk before apply the hunk.
(diff-hunk-kill, diff-file-kill): Jump to beginning of hunk after kill.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
---
 lisp/vc/diff-mode.el | 241 ++++++++++++++++++---------------------------------
 1 file changed, 83 insertions(+), 158 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 9dfcd944bb..3fc4713f0f 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -498,22 +498,55 @@ diff-end-of-hunk
     ;; The return value is used by easy-mmode-define-navigation.
     (goto-char (or end (point-max)))))
 
+;; "index ", "old mode", "new mode", "new file mode" and
+;; "deleted file mode" are output by git-diff.
+(defconst diff-file-junk-re
+  "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+
+;; If point is in a diff header, then return beginning
+;; of hunk position otherwise return nil.
+(defun diff--at-diff-header-p ()
+  "Return non-nil if point is inside a diff header."
+  (let ((regexp-hunk diff-hunk-header-re)
+        (regexp-file diff-file-header-re)
+        (regexp-junk diff-file-junk-re)
+        (orig (point)))
+    (catch 'headerp
+      (save-excursion
+        (forward-line 0)
+        (when (looking-at regexp-hunk) ; Hunk header.
+          (throw 'headerp (point)))
+        (forward-line -1)
+        (when (re-search-forward regexp-file (point-at-eol 4) t) ; File header.
+          (forward-line 0)
+          (throw 'headerp (point)))
+        (goto-char orig)
+        (forward-line 0)
+        (when (looking-at regexp-junk) ; Git diff junk.
+          (while (and (looking-at regexp-junk)
+                      (not (bobp)))
+            (forward-line -1))
+          (re-search-forward regexp-file nil t)
+          (forward-line 0)
+          (throw 'headerp (point)))) nil)))
+
 (defun diff-beginning-of-hunk (&optional try-harder)
   "Move back to the previous hunk beginning, and return its position.
 If point is in a file header rather than a hunk, advance to the
 next hunk if TRY-HARDER is non-nil; otherwise signal an error."
   (beginning-of-line)
-  (if (looking-at diff-hunk-header-re)
+  (if (looking-at diff-hunk-header-re) ; At hunk header.
       (point)
-    (forward-line 1)
-    (condition-case ()
-	(re-search-backward diff-hunk-header-re)
-      (error
-       (unless try-harder
-	 (error "Can't find the beginning of the hunk"))
-       (diff-beginning-of-file-and-junk)
-       (diff-hunk-next)
-       (point)))))
+    (let ((pos (diff--at-diff-header-p))
+          (regexp diff-hunk-header-re))
+      (cond (pos ; At junk diff header.
+             (if try-harder
+                 (goto-char pos)
+               (error "Can't find the beginning of the hunk")))
+            ((re-search-backward regexp nil t)) ; In the middle of a hunk.
+            ((re-search-forward regexp nil t) ; At first hunk header.
+             (forward-line 0))
+            (t (error "Can't find the beginning of the hunk"))))))
 
 (defun diff-unified-hunk-p ()
   (save-excursion
@@ -551,124 +584,26 @@ diff--auto-refine-data
 
 ;; Define diff-{hunk,file}-{prev,next}
 (easy-mmode-define-navigation
- diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view)
+ diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view
+ (when diff-auto-refine-mode
+   (unless (prog1 diff--auto-refine-data
+             (setq diff--auto-refine-data
+                   (cons (current-buffer) (point-marker))))
+     (run-at-time 0.0 nil
+                  (lambda ()
+                    (when diff--auto-refine-data
+                      (let ((buffer (car diff--auto-refine-data))
+                            (point (cdr diff--auto-refine-data)))
+                        (setq diff--auto-refine-data nil)
+                        (with-local-quit
+                          (when (buffer-live-p buffer)
+                            (with-current-buffer buffer
+                              (save-excursion
+                                (goto-char point)
+                                (diff-refine-hunk))))))))))))
 
 (easy-mmode-define-navigation
- diff--internal-file diff-file-header-re "file" diff-end-of-file)
-
-(defun diff--wrap-navigation (skip-hunk-start
-                              what orig
-                              header-re goto-start-func count)
-  "Wrap diff-{hunk,file}-{next,prev} for more intuitive behavior.
-Override the default diff-{hunk,file}-{next,prev} implementation
-by skipping any lines that are associated with this hunk/file 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.  Move the point to the @@... marker before
-executing the default diff-hunk-next/prev implementation to move
-to the NEXT marker."
-  (if (not skip-hunk-start)
-      (funcall orig count)
-
-    (let ((start (point)))
-      (funcall goto-start-func)
-
-      ;; Trap the error.
-      (condition-case nil
-          (funcall orig count)
-        (error nil))
-
-      (when (not (looking-at header-re))
-        (goto-char start)
-        (user-error (format "No %s" what)))
-
-      ;; We successfully moved to the next/prev hunk/file. Apply the
-      ;; auto-refinement if needed
-      (when diff-auto-refine-mode
-        (unless (prog1 diff--auto-refine-data
-                  (setq diff--auto-refine-data
-                        (cons (current-buffer) (point-marker))))
-          (run-at-time 0.0 nil
-                       (lambda ()
-                         (when diff--auto-refine-data
-                           (let ((buffer (car diff--auto-refine-data))
-                                 (point (cdr diff--auto-refine-data)))
-                             (setq diff--auto-refine-data nil)
-                             (with-local-quit
-                               (when (buffer-live-p buffer)
-                                 (with-current-buffer buffer
-                                   (save-excursion
-                                     (goto-char point)
-                                     (diff-refine-hunk))))))))))))))
-
-;; These functions all take a skip-hunk-start argument which controls
-;; whether we skip pre-hunk-start text or not.  In interactive uses we
-;; always want to do this, but the simple behavior is still necessary
-;; to, for example, avoid an infinite loop:
-;;
-;;   diff-hunk-next         calls
-;;   diff--wrap-navigation  calls
-;;   diff-bounds-of-hunk    calls
-;;   diff-beginning-of-hunk calls
-;;   diff-hunk-next
-;;
-;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the
-;; inner one does not, which breaks the loop.
-(defun diff-hunk-prev (&optional count skip-hunk-start)
-  "Go to the previous COUNT'th hunk."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "prev hunk"
-   'diff--internal-hunk-prev
-   diff-hunk-header-re
-   (lambda () (goto-char (car (diff-bounds-of-hunk))))
-   count))
-
-(defun diff-hunk-next (&optional count skip-hunk-start)
-  "Go to the next COUNT'th hunk."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "next hunk"
-   'diff--internal-hunk-next
-   diff-hunk-header-re
-   (lambda () (goto-char (car (diff-bounds-of-hunk))))
-   count))
-
-(defun diff-file-prev (&optional count skip-hunk-start)
-  "Go to the previous COUNT'th file."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "prev file"
-   'diff--internal-file-prev
-   diff-file-header-re
-   (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
-   count))
-
-(defun diff-file-next (&optional count skip-hunk-start)
-  "Go to the next COUNT'th file."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "next file"
-   'diff--internal-file-next
-   diff-file-header-re
-   (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
-   count))
-
-
-
+ diff-file diff-file-header-re "file" diff-end-of-file)
 
 (defun diff-bounds-of-hunk ()
   "Return the bounds of the diff hunk at point.
@@ -679,13 +614,12 @@ diff-bounds-of-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)))
-	    ;; There's no next hunk, so just take the one we have.
-	    (t (list beg end))))))
+	    (t (error "No hunk found"))))))
 
 (defun diff-bounds-of-file ()
   "Return the bounds of the file segment at point.
@@ -731,12 +665,8 @@ diff-hunk-kill
 		   hunk-bounds))
 	 (inhibit-read-only t))
     (apply 'kill-region bounds)
-    (goto-char (car bounds))))
-
-;; "index ", "old mode", "new mode", "new file mode" and
-;; "deleted file mode" are output by git-diff.
-(defconst diff-file-junk-re
-  "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+    (goto-char (car bounds))
+    (diff-beginning-of-hunk t)))
 
 (defun diff-beginning-of-file-and-junk ()
   "Go to the beginning of file-related diff-info.
@@ -771,7 +701,7 @@ diff-beginning-of-file-and-junk
         (setq prevfile nextfile))
     (if (and previndex (numberp prevfile) (< previndex prevfile))
         (setq prevfile previndex))
-    (if (numberp prevfile)
+    (if (and (numberp prevfile) (<= prevfile start))
           (progn
             (goto-char prevfile)
             ;; Now skip backward over the leading junk we may have before the
@@ -789,7 +719,8 @@ diff-file-kill
   "Kill current file's hunks."
   (interactive)
   (let ((inhibit-read-only t))
-    (apply 'kill-region (diff-bounds-of-file))))
+    (apply 'kill-region (diff-bounds-of-file)))
+  (diff-beginning-of-hunk t))
 
 (defun diff-kill-junk ()
   "Kill spurious empty diffs."
@@ -1373,7 +1304,7 @@ diff-post-command-hook
 	;; it's safer not to do it on big changes, e.g. when yanking a big
 	;; diff, or when the user edits the header, since we might then
 	;; screw up perfectly correct values.  --Stef
-	(diff-beginning-of-hunk)
+	(diff-beginning-of-hunk t)
         (let* ((style (if (looking-at "\\*\\*\\*") 'context))
                (start (line-beginning-position (if (eq style 'context) 3 2)))
                (mid (if (eq style 'context)
@@ -1764,9 +1695,8 @@ diff-find-source-location
 SWITCHED is non-nil if the patch is already applied.
 NOPROMPT, if non-nil, means not to prompt the user."
   (save-excursion
-    (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))))
+    (let* ((other (diff-xor other-file diff-jump-to-old-file))
+	   (char-offset (- (point) (diff-beginning-of-hunk t)))
            ;; 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),
@@ -1775,7 +1705,7 @@ diff-find-source-location
 	   ;; Suppress check when NOPROMPT is non-nil (Bug#3033).
            (_ (unless noprompt (diff-sanity-check-hunk)))
 	   (hunk (buffer-substring
-                  (point) (cadr hunk-bounds)))
+                  (point) (save-excursion (diff-end-of-hunk) (point))))
 	   (old (diff-hunk-text hunk reverse char-offset))
 	   (new (diff-hunk-text hunk (not reverse) char-offset))
 	   ;; Find the location specification.
@@ -1838,6 +1768,7 @@ diff-apply-hunk
 
 With a prefix argument, REVERSE the hunk."
   (interactive "P")
+  (diff-beginning-of-hunk t)
   (pcase-let ((`(,buf ,line-offset ,pos ,old ,new ,switched)
                ;; Sometimes we'd like to have the following behavior: if
                ;; REVERSE go to the new file, otherwise go to the old.
@@ -1883,15 +1814,8 @@ diff-apply-hunk
       ;; Display BUF in a window
       (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
       (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil)
-
-      ;; Advance to the next hunk with skip-hunk-start set to t
-      ;; because we want the behavior of moving to the next logical
-      ;; hunk, not the original behavior where were would sometimes
-      ;; stay on the current hunk.  This is the behavior we get when
-      ;; navigating through hunks interactively, and we want it when
-      ;; applying hunks too (see http://debbugs.gnu.org/17544).
       (when diff-advance-after-apply-hunk
-	(diff-hunk-next nil t))))))
+	(diff-hunk-next))))))
 
 
 (defun diff-test-hunk (&optional reverse)
@@ -1972,15 +1896,14 @@ diff-current-defun
 (defun diff-ignore-whitespace-hunk ()
   "Re-diff the current hunk, ignoring whitespace differences."
   (interactive)
-  (let* ((hunk-bounds (diff-bounds-of-hunk))
-         (char-offset (- (point) (goto-char (car hunk-bounds))))
+  (let* ((char-offset (- (point) (diff-beginning-of-hunk t)))
 	 (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) (cadr hunk-bounds)))
+		(point) (save-excursion (diff-end-of-hunk) (point))))
 	 (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1.
 	 (file1 (make-temp-file "diff1"))
 	 (file2 (make-temp-file "diff2"))
@@ -2067,14 +1990,16 @@ diff-refine-hunk
   (interactive)
   (require 'smerge-mode)
   (save-excursion
-    (let* ((hunk-bounds (diff-bounds-of-hunk))
-           (style (progn (goto-char (car hunk-bounds))
-                         (diff-hunk-style))) ;Skips the hunk header as well.
+    (diff-beginning-of-hunk t)
+    (let* ((start (point))
+           (style (diff-hunk-style))    ;Skips the hunk header as well.
            (beg (point))
-           (end (cadr hunk-bounds))
            (props-c '((diff-mode . fine) (face diff-refine-changed)))
            (props-r '((diff-mode . fine) (face diff-refine-removed)))
-           (props-a '((diff-mode . fine) (face diff-refine-added))))
+           (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))))
 
       (remove-overlays beg end 'diff-mode 'fine)
 
-- 
2.11.0

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.5)
 of 2017-01-10
Repository revision: fa0a2b4e7c81f57aecc1d94df00588a4dd5c281d

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

From: Mark Oteiza <mvoteiza <at> udel.edu>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: Glenn Morris <rgm <at> gnu.org>, 25400 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net, Eli Zaretskii <eliz <at> gnu.org>,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>, Dmitry Gutov <dgutov <at> yandex.ru>,
 25105 <at> debbugs.gnu.org, Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25400: M-p in diff-mode jumps too far
Date: Wed, 11 Jan 2017 18:27:55 -0500

On 11/01/17 at 01:38pm, Tino Calancha wrote:
> Stefan Monnier <monnier <at> IRO.UMontreal.CA> writes:
> 
> > In a buffer with more than one hunk, if I'm in the middle of hunk number
> > N, diff-hunk-prev (usually bound to M-p) jumps to the header of hunk
> > number N-1 rather than to the header of hunk N.
> >
> > This is contrary to the usual behavior of Emacs's navigation commands.
> >As pointed out elsewhere, it's particularly obnoxious from EOB (in which
> >case, you're not really "within N" but you're virtually on "the header
> >of the non-existent hunk N+1", so going to the header of N-1 is really
> >wrong).
> >
> >I also dislike the fact that M-n doesn't let me get to EOB.
> 
> Following patch reverts commit 2c8a7e5.  Then it fixes dots 1. and 2.
> described in the commit message of 2c8a7e5, i.e., Bug#17544.
> This patch preserves the original definitions for 'diff-hunk-prev'
> and 'diff-hunk-next'.
> 
> After applying locally this patch, you might want to do:
> git diff 2c8a7e5^ HEAD lisp/vc/diff-mode.el
> to see more clearly how it solves Bug#17544.

Works for me AFAICT.  Thanks for working on it.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tino Calancha <tino.calancha <at> gmail.com>,
 Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Cc: Mark Oteiza <mvoteiza <at> udel.edu>, 25400 <at> debbugs.gnu.org,
 Dima Kogan <dima <at> secretsauce.net>, 25105 <at> debbugs.gnu.org,
 npostavs <at> users.sourceforge.net
Subject: Re: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Thu, 12 Jan 2017 02:34:39 +0300

Hi Tino,

Thanks for doing this. However, I'd prefer two separate commits: one 
reverting 2c8a7e5 (if that is what we are doing), and one with your 
actual changes.

That would make the history easier to read, and would probably improve 
'git blame' results as well.

Thanks.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 25400 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net,
 Tino Calancha <tino.calancha <at> gmail.com>, Mark Oteiza <mvoteiza <at> udel.edu>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 25105 <at> debbugs.gnu.org,
 Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Thu, 12 Jan 2017 12:53:48 +0900 (JST)

On Thu, 12 Jan 2017, Dmitry Gutov wrote:

> I'd prefer two separate commits:
> one reverting 2c8a7e5 (if that is what we are doing),
> and one with your actual changes.
>
> That would make the history easier to read, and would probably improve 'git 
> blame' results as well.
Definitely.  Thanks for the suggestion!

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
From 4c0b6cb87438a5c812a4718452c2537d827a74a4 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Thu, 12 Jan 2017 12:46:03 +0900
Subject: [PATCH 1/2] ; Revert "Improve diff-mode navigation/manipulation"

This reverts commit 2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085.
This change causes regressions:
https://lists.gnu.org/archive/html/emacs-devel/2016-11/msg00738.html

Fixes: debbugs:25105, 25400.
---
 lisp/vc/diff-mode.el | 174 ++++++++++-----------------------------------------
 1 file changed, 34 insertions(+), 140 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 9dfcd944bb..44556ddd4a 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -551,124 +551,26 @@ diff--auto-refine-data

 ;; Define diff-{hunk,file}-{prev,next}
 (easy-mmode-define-navigation
- diff--internal-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view)
+ diff-hunk diff-hunk-header-re "hunk" diff-end-of-hunk diff-restrict-view
+ (when diff-auto-refine-mode
+   (unless (prog1 diff--auto-refine-data
+             (setq diff--auto-refine-data
+                   (cons (current-buffer) (point-marker))))
+     (run-at-time 0.0 nil
+                  (lambda ()
+                    (when diff--auto-refine-data
+                      (let ((buffer (car diff--auto-refine-data))
+                            (point (cdr diff--auto-refine-data)))
+                        (setq diff--auto-refine-data nil)
+                        (with-local-quit
+                          (when (buffer-live-p buffer)
+                            (with-current-buffer buffer
+                              (save-excursion
+                                (goto-char point)
+                                (diff-refine-hunk))))))))))))

 (easy-mmode-define-navigation
- diff--internal-file diff-file-header-re "file" diff-end-of-file)
-
-(defun diff--wrap-navigation (skip-hunk-start
-                              what orig
-                              header-re goto-start-func count)
-  "Wrap diff-{hunk,file}-{next,prev} for more intuitive behavior.
-Override the default diff-{hunk,file}-{next,prev} implementation
-by skipping any lines that are associated with this hunk/file 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.  Move the point to the @@... marker before
-executing the default diff-hunk-next/prev implementation to move
-to the NEXT marker."
-  (if (not skip-hunk-start)
-      (funcall orig count)
-
-    (let ((start (point)))
-      (funcall goto-start-func)
-
-      ;; Trap the error.
-      (condition-case nil
-          (funcall orig count)
-        (error nil))
-
-      (when (not (looking-at header-re))
-        (goto-char start)
-        (user-error (format "No %s" what)))
-
-      ;; We successfully moved to the next/prev hunk/file. Apply the
-      ;; auto-refinement if needed
-      (when diff-auto-refine-mode
-        (unless (prog1 diff--auto-refine-data
-                  (setq diff--auto-refine-data
-                        (cons (current-buffer) (point-marker))))
-          (run-at-time 0.0 nil
-                       (lambda ()
-                         (when diff--auto-refine-data
-                           (let ((buffer (car diff--auto-refine-data))
-                                 (point (cdr diff--auto-refine-data)))
-                             (setq diff--auto-refine-data nil)
-                             (with-local-quit
-                               (when (buffer-live-p buffer)
-                                 (with-current-buffer buffer
-                                   (save-excursion
-                                     (goto-char point)
-                                     (diff-refine-hunk))))))))))))))
-
-;; These functions all take a skip-hunk-start argument which controls
-;; whether we skip pre-hunk-start text or not.  In interactive uses we
-;; always want to do this, but the simple behavior is still necessary
-;; to, for example, avoid an infinite loop:
-;;
-;;   diff-hunk-next         calls
-;;   diff--wrap-navigation  calls
-;;   diff-bounds-of-hunk    calls
-;;   diff-beginning-of-hunk calls
-;;   diff-hunk-next
-;;
-;; Here the outer diff-hunk-next has skip-hunk-start set to t, but the
-;; inner one does not, which breaks the loop.
-(defun diff-hunk-prev (&optional count skip-hunk-start)
-  "Go to the previous COUNT'th hunk."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "prev hunk"
-   'diff--internal-hunk-prev
-   diff-hunk-header-re
-   (lambda () (goto-char (car (diff-bounds-of-hunk))))
-   count))
-
-(defun diff-hunk-next (&optional count skip-hunk-start)
-  "Go to the next COUNT'th hunk."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "next hunk"
-   'diff--internal-hunk-next
-   diff-hunk-header-re
-   (lambda () (goto-char (car (diff-bounds-of-hunk))))
-   count))
-
-(defun diff-file-prev (&optional count skip-hunk-start)
-  "Go to the previous COUNT'th file."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "prev file"
-   'diff--internal-file-prev
-   diff-file-header-re
-   (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
-   count))
-
-(defun diff-file-next (&optional count skip-hunk-start)
-  "Go to the next COUNT'th file."
-  (interactive (list (prefix-numeric-value current-prefix-arg) t))
-  (diff--wrap-navigation
-   skip-hunk-start
-   "next file"
-   'diff--internal-file-next
-   diff-file-header-re
-   (lambda () (goto-char (car (diff-bounds-of-file))) (diff--internal-hunk-next))
-   count))
-
-
-
+ diff-file diff-file-header-re "file" diff-end-of-file)

 (defun diff-bounds-of-hunk ()
   "Return the bounds of the diff hunk at point.
@@ -679,13 +581,12 @@ diff-bounds-of-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)))
-	    ;; There's no next hunk, so just take the one we have.
-	    (t (list beg end))))))
+	    (t (error "No hunk found"))))))

 (defun diff-bounds-of-file ()
   "Return the bounds of the file segment at point.
@@ -771,7 +672,7 @@ diff-beginning-of-file-and-junk
         (setq prevfile nextfile))
     (if (and previndex (numberp prevfile) (< previndex prevfile))
         (setq prevfile previndex))
-    (if (numberp prevfile)
+    (if (and (numberp prevfile) (<= prevfile start))
           (progn
             (goto-char prevfile)
             ;; Now skip backward over the leading junk we may have before the
@@ -1764,9 +1665,8 @@ diff-find-source-location
 SWITCHED is non-nil if the patch is already applied.
 NOPROMPT, if non-nil, means not to prompt the user."
   (save-excursion
-    (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))))
+    (let* ((other (diff-xor other-file diff-jump-to-old-file))
+	   (char-offset (- (point) (diff-beginning-of-hunk t)))
            ;; 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),
@@ -1775,7 +1675,7 @@ diff-find-source-location
 	   ;; Suppress check when NOPROMPT is non-nil (Bug#3033).
            (_ (unless noprompt (diff-sanity-check-hunk)))
 	   (hunk (buffer-substring
-                  (point) (cadr hunk-bounds)))
+                  (point) (save-excursion (diff-end-of-hunk) (point))))
 	   (old (diff-hunk-text hunk reverse char-offset))
 	   (new (diff-hunk-text hunk (not reverse) char-offset))
 	   ;; Find the location specification.
@@ -1883,15 +1783,8 @@ diff-apply-hunk
       ;; Display BUF in a window
       (set-window-point (display-buffer buf) (+ (car pos) (cdr new)))
       (diff-hunk-status-msg line-offset (diff-xor switched reverse) nil)
-
-      ;; Advance to the next hunk with skip-hunk-start set to t
-      ;; because we want the behavior of moving to the next logical
-      ;; hunk, not the original behavior where were would sometimes
-      ;; stay on the current hunk.  This is the behavior we get when
-      ;; navigating through hunks interactively, and we want it when
-      ;; applying hunks too (see http://debbugs.gnu.org/17544).
       (when diff-advance-after-apply-hunk
-	(diff-hunk-next nil t))))))
+	(diff-hunk-next))))))


 (defun diff-test-hunk (&optional reverse)
@@ -1972,15 +1865,14 @@ diff-current-defun
 (defun diff-ignore-whitespace-hunk ()
   "Re-diff the current hunk, ignoring whitespace differences."
   (interactive)
-  (let* ((hunk-bounds (diff-bounds-of-hunk))
-         (char-offset (- (point) (goto-char (car hunk-bounds))))
+  (let* ((char-offset (- (point) (diff-beginning-of-hunk t)))
 	 (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) (cadr hunk-bounds)))
+		(point) (save-excursion (diff-end-of-hunk) (point))))
 	 (lead (make-string (1- line-nb) ?\n)) ;Line nums start at 1.
 	 (file1 (make-temp-file "diff1"))
 	 (file2 (make-temp-file "diff2"))
@@ -2067,14 +1959,16 @@ diff-refine-hunk
   (interactive)
   (require 'smerge-mode)
   (save-excursion
-    (let* ((hunk-bounds (diff-bounds-of-hunk))
-           (style (progn (goto-char (car hunk-bounds))
-                         (diff-hunk-style))) ;Skips the hunk header as well.
+    (diff-beginning-of-hunk t)
+    (let* ((start (point))
+           (style (diff-hunk-style))    ;Skips the hunk header as well.
            (beg (point))
-           (end (cadr hunk-bounds))
            (props-c '((diff-mode . fine) (face diff-refine-changed)))
            (props-r '((diff-mode . fine) (face diff-refine-removed)))
-           (props-a '((diff-mode . fine) (face diff-refine-added))))
+           (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))))

       (remove-overlays beg end 'diff-mode 'fine)

-- 
2.11.0

From a5809529fac90741c6ede3fc66dfdac1e011434c Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Thu, 12 Jan 2017 12:46:25 +0900
Subject: [PATCH 2/2] Fix Bug#17544

* lisp/vc/diff-mode.el (diff-file-junk-re):
Move definition before it's used.
(diff--at-diff-header-p): New predicate.
(diff-beginning-of-hunk): Use it.
(diff-apply-hunk): Jump to beginning of hunk before apply the hunk.
(diff-hunk-kill, diff-file-kill): Jump to beginning of hunk after kill.
(diff-post-command-hook): Call diff-beginning-of-hunk with non-nil argument.
---
 lisp/vc/diff-mode.el | 67 ++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 49 insertions(+), 18 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 44556ddd4a..3fc4713f0f 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -498,22 +498,55 @@ diff-end-of-hunk
     ;; The return value is used by easy-mmode-define-navigation.
     (goto-char (or end (point-max)))))

+;; "index ", "old mode", "new mode", "new file mode" and
+;; "deleted file mode" are output by git-diff.
+(defconst diff-file-junk-re
+  "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+
+;; If point is in a diff header, then return beginning
+;; of hunk position otherwise return nil.
+(defun diff--at-diff-header-p ()
+  "Return non-nil if point is inside a diff header."
+  (let ((regexp-hunk diff-hunk-header-re)
+        (regexp-file diff-file-header-re)
+        (regexp-junk diff-file-junk-re)
+        (orig (point)))
+    (catch 'headerp
+      (save-excursion
+        (forward-line 0)
+        (when (looking-at regexp-hunk) ; Hunk header.
+          (throw 'headerp (point)))
+        (forward-line -1)
+        (when (re-search-forward regexp-file (point-at-eol 4) t) ; File header.
+          (forward-line 0)
+          (throw 'headerp (point)))
+        (goto-char orig)
+        (forward-line 0)
+        (when (looking-at regexp-junk) ; Git diff junk.
+          (while (and (looking-at regexp-junk)
+                      (not (bobp)))
+            (forward-line -1))
+          (re-search-forward regexp-file nil t)
+          (forward-line 0)
+          (throw 'headerp (point)))) nil)))
+
 (defun diff-beginning-of-hunk (&optional try-harder)
   "Move back to the previous hunk beginning, and return its position.
 If point is in a file header rather than a hunk, advance to the
 next hunk if TRY-HARDER is non-nil; otherwise signal an error."
   (beginning-of-line)
-  (if (looking-at diff-hunk-header-re)
+  (if (looking-at diff-hunk-header-re) ; At hunk header.
       (point)
-    (forward-line 1)
-    (condition-case ()
-	(re-search-backward diff-hunk-header-re)
-      (error
-       (unless try-harder
-	 (error "Can't find the beginning of the hunk"))
-       (diff-beginning-of-file-and-junk)
-       (diff-hunk-next)
-       (point)))))
+    (let ((pos (diff--at-diff-header-p))
+          (regexp diff-hunk-header-re))
+      (cond (pos ; At junk diff header.
+             (if try-harder
+                 (goto-char pos)
+               (error "Can't find the beginning of the hunk")))
+            ((re-search-backward regexp nil t)) ; In the middle of a hunk.
+            ((re-search-forward regexp nil t) ; At first hunk header.
+             (forward-line 0))
+            (t (error "Can't find the beginning of the hunk"))))))

 (defun diff-unified-hunk-p ()
   (save-excursion
@@ -632,12 +665,8 @@ diff-hunk-kill
 		   hunk-bounds))
 	 (inhibit-read-only t))
     (apply 'kill-region bounds)
-    (goto-char (car bounds))))
-
-;; "index ", "old mode", "new mode", "new file mode" and
-;; "deleted file mode" are output by git-diff.
-(defconst diff-file-junk-re
-  "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file")
+    (goto-char (car bounds))
+    (diff-beginning-of-hunk t)))

 (defun diff-beginning-of-file-and-junk ()
   "Go to the beginning of file-related diff-info.
@@ -690,7 +719,8 @@ diff-file-kill
   "Kill current file's hunks."
   (interactive)
   (let ((inhibit-read-only t))
-    (apply 'kill-region (diff-bounds-of-file))))
+    (apply 'kill-region (diff-bounds-of-file)))
+  (diff-beginning-of-hunk t))

 (defun diff-kill-junk ()
   "Kill spurious empty diffs."
@@ -1274,7 +1304,7 @@ diff-post-command-hook
 	;; it's safer not to do it on big changes, e.g. when yanking a big
 	;; diff, or when the user edits the header, since we might then
 	;; screw up perfectly correct values.  --Stef
-	(diff-beginning-of-hunk)
+	(diff-beginning-of-hunk t)
         (let* ((style (if (looking-at "\\*\\*\\*") 'context))
                (start (line-beginning-position (if (eq style 'context) 3 2)))
                (mid (if (eq style 'context)
@@ -1738,6 +1768,7 @@ diff-apply-hunk

 With a prefix argument, REVERSE the hunk."
   (interactive "P")
+  (diff-beginning-of-hunk t)
   (pcase-let ((`(,buf ,line-offset ,pos ,old ,new ,switched)
                ;; Sometimes we'd like to have the following behavior: if
                ;; REVERSE go to the new file, otherwise go to the old.
-- 
2.11.0

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
In GNU Emacs 26.0.50.1 (x86_64-pc-linux-gnu, GTK+ Version 3.22.5)
 of 2017-01-12
Repository revision: d40073f017ffb3dee2266f356c127ef587c40b71

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 25400 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net,
 Mark Oteiza <mvoteiza <at> udel.edu>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 25105 <at> debbugs.gnu.org, Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Fri, 13 Jan 2017 06:35:20 +0300

On 12.01.2017 06:53, Tino Calancha wrote:

> Definitely.  Thanks for the suggestion!

Could you re-send the patches at attachments? Or just push them to a 
branch. I wish the first one could be produced with a simple 'git 
revert', but that leads to a conflict.

I get "Can't find the text to patch" with the second one, though.

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 25400 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net,
 Tino Calancha <tino.calancha <at> gmail.com>, Mark Oteiza <mvoteiza <at> udel.edu>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 25105 <at> debbugs.gnu.org,
 Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Fri, 13 Jan 2017 12:55:40 +0900 (JST)

On Fri, 13 Jan 2017, Dmitry Gutov wrote:

> Could you re-send the patches at attachments? Or just push them to a branch. 
I have pushed those changes into following new branch:
scratch/calancha-revert-2c8a7e5

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

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 25400 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net,
 Tino Calancha <tino.calancha <at> gmail.com>, Mark Oteiza <mvoteiza <at> udel.edu>,
 Stefan Monnier <monnier <at> iro.umontreal.ca>, 25105 <at> debbugs.gnu.org,
 Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Fri, 13 Jan 2017 13:25:42 +0900 (JST)

On Fri, 13 Jan 2017, Dmitry Gutov wrote:

> I wish the first one could be produced with a simple 'git revert', but that 
> leads to a conflict.
Sorry for the confusin.  I made the revert manually.
In addition to commit 2c8a7e5,  i've reverted following
other related commits:

e5ef59b87da5c2ddfa22f7342efe29b3eea6ed97
6b6abe0dba6a9a2e5f78aac3814421886e7a184f
73349822cbd6e50526eda9c75453584d73dfca83
a283d655db88cdcc8cb53d8e2578e1cdf751c84b
61c6a10e3110490dadac4577cc540053341ff25c
2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085


In scratch/calancha-revert-2c8a7e5 branch,
diff-mode.el history is as follows:

##### _before_ my changes #####
;; initial state for diff-mode.el is as in commit:
1f5592572887fe15e5b660bc60e66a7ab7c624cd ; w/ copyright year 2016
;; Update copyright year and we are at 
;; 3e30cda89474209716c6e16a1a81d02877c95a2b
;; i.e., first commit in my new branch).

;; then, second commit (0beb7d2968ab76878eb3be26f2d749977fdcaa2f)
;; add my fix for Bug#17544.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 25400 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net,
 Mark Oteiza <mvoteiza <at> udel.edu>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 25105 <at> debbugs.gnu.org, Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25400: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Sat, 14 Jan 2017 06:11:06 +0300

On 13.01.2017 06:55, Tino Calancha wrote:

> I have pushed those changes into following new branch:
> scratch/calancha-revert-2c8a7e5

I've tried it out (but not the changes discussed in this thread later), 
and the behavior is much better. Thanks!

Please apply it to master sooner rather than later.

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

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 25400 <at> debbugs.gnu.org, npostavs <at> users.sourceforge.net,
 Mark Oteiza <mvoteiza <at> udel.edu>, Stefan Monnier <monnier <at> iro.umontreal.ca>,
 25105 <at> debbugs.gnu.org, Dima Kogan <dima <at> secretsauce.net>
Subject: Re: bug#25105: bug#25400: M-p in diff-mode jumps too far
Date: Sat, 14 Jan 2017 06:16:45 +0300

On 13.01.2017 07:25, Tino Calancha wrote:

> Sorry for the confusin.  I made the revert manually.
> In addition to commit 2c8a7e5,  i've reverted following
> other related commits:
>
> e5ef59b87da5c2ddfa22f7342efe29b3eea6ed97
> 6b6abe0dba6a9a2e5f78aac3814421886e7a184f
> 73349822cbd6e50526eda9c75453584d73dfca83
> a283d655db88cdcc8cb53d8e2578e1cdf751c84b
> 61c6a10e3110490dadac4577cc540053341ff25c
> 2c8a7e50d24daf19ea7d86f1cfeaa98a41c56085

On that subject, maybe reverting each of them separately would be better.

Although I've tried how that affects 'git blame' in practice, and it 
makes no difference.

Maybe our Changelog-generating script could take reverts into account 
(and skip commits that have been explicitly reverted later), if it 
doesn't do that already.

Message #47 received at 25400-done <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 25105-done <at> debbugs.gnu.org, 25400-done <at> debbugs.gnu.org
Subject: Re: bug#25400: bug#25105: bug#25400: M-p in diff-mode jumps too
 far
Date: Sat, 21 Jan 2017 12:02:34 +0900 (JST)

On Sat, 14 Jan 2017, Dmitry Gutov wrote:

> On 13.01.2017 06:55, Tino Calancha wrote:
>
>> I have pushed those changes into following new branch:
>> scratch/calancha-revert-2c8a7e5
>
> I've tried it out (but not the changes discussed in this thread later), and 
> the behavior is much better. Thanks!
>
> Please apply it to master sooner rather than later.
Pushed the fix to master branch as commits:
1508b538fd8f8c2e00aadcea42ac36013fad02e3
e5e42cefd7f2eb47d2c8660a7a317e8b08d36a82

Previous Next

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