Subject: Re: bug#32991: 27.0.50; diff-auto-refine-mode a no-op

From: [email-masked] (Charles A. Roelli)
Date: Sun, 13 Jan 2019 21:03:29 +0100

> Date: Sun, 13 Jan 2019 15:36:48 +0100
> From: [email-masked] (Charles A. Roelli)
> 
> After revisiting this, the values "nil", "font-lock", and "navigation"
> sound fine, though if we end up using those we might want to change
> the name of "diff-font-lock-refine" to just "diff-refine" (since
> "font-lock" may not be involved in the refinement).

Here is a possible implementation:

* etc/NEWS (Diff mode): Explain renamed 'diff-refine' variable,
mention removal of 'diff-auto-refine-mode', and explain
user-visible changes to 'diff-hunk-next' and 'diff-hunk-prev'.
* lisp/vc/diff-mode.el (diff-font-lock-refine): Rename to
'diff-refine' and allow choices nil, 'font-lock' and 'navigation'.
(diff-auto-refine-mode): Remove as it has been superseded by the
user option 'diff-refine'.
(diff-navigate-maybe-refine): New function for use in the new
functions 'diff-hunk-next' and 'diff-hunk-prev'.
(diff-hunk-next, diff-hunk-prev): Rewrite as defuns instead of
using easy-mmode-define-navigation.  Add a new optional argument
'interactive', which is needed to prevent other callers from
inadvertently causing refinement to happen when 'diff-refine' is
set to 'navigation'.  Leave out the ability of these commands to
change user narrowing and recenter the window, as this does not
match the behavior of other general movement commands.
(diff--font-lock-refined): Change it to only trigger when the new
variable 'diff-refine' has the value 'font-lock'.
* lisp/vc/smerge-mode.el (smerge-next, smerge-prev): Check
that 'diff-refine' is set to 'navigation' instead of checking
'diff-auto-refine-mode' when deciding whether to refine a
conflict.
------

diff --git a/etc/NEWS b/etc/NEWS
index bb214f2..3163577 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -428,8 +428,14 @@ values.
 and compares their entire trees.
 
 ** Diff mode
-*** Hunks are now automatically refined by default.
-To disable it, set the new defcustom 'diff-font-lock-refine' to nil.
+*** Hunks are now automatically refined by font-lock.
+To disable refinement, set the new defcustom 'diff-refine' to nil.
+To get back the old behavior where hunks are refined as you navigate
+through a diff, set 'diff-refine' to the symbol 'navigate'.
+*** diff-auto-refine-mode has been removed.
+*** diff-hunk-next and diff-hunk-prev
+These commands no longer recenter the window or change the narrowing
+of the buffer.
 
 +++
 *** Better syntax highlighting of Diff hunks.
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 5d6cc6f..2d77f6a 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -94,10 +94,18 @@ diff-mode-hook
   :type 'hook
   :options '(diff-delete-empty-files diff-make-unified))
 
-(defcustom diff-font-lock-refine t
-  "If non-nil, font-lock highlighting includes hunk refinement."
+(defcustom diff-refine 'font-lock
+  "If non-nil, enable hunk refinement.
+
+The value `font-lock' means to refine during font-lock.
+The value `navigation' means to refine each hunk as you visit it
+with `diff-hunk-next' or `diff-hunk-prev'.
+
+You can always manually refine a hunk with `diff-refine-hunk'."
   :version "27.1"
-  :type 'boolean)
+  :type '(choice (const :tag "Don't refine hunks" nil)
+                 (const :tag "Refine hunks during font-lock" font-lock)
+                 (const :tag "Refine hunks during navigation" navigation)))
 
 (defcustom diff-font-lock-prettify nil
   "If non-nil, font-lock will try and make the format prettier."
@@ -254,18 +262,6 @@ diff-minor-mode-prefix
   `((,diff-minor-mode-prefix . ,diff-mode-shared-map))
   "Keymap for `diff-minor-mode'.  See also `diff-mode-shared-map'.")
 
-(define-minor-mode diff-auto-refine-mode
-  "Toggle automatic diff hunk finer highlighting (Diff Auto Refine mode).
-
-Diff Auto Refine mode is a buffer-local minor mode used with
-`diff-mode'.  When enabled, Emacs automatically highlights
-changes in detail as the user visits hunks.  When transitioning
-from disabled to enabled, it tries to refine the current hunk, as
-well."
-  :group 'diff-mode :init-value t :lighter nil ;; " Auto-Refine"
-  (when diff-auto-refine-mode
-    (condition-case-unless-debug nil (diff-refine-hunk) (error nil))))
-
 ;;;;
 ;;;; font-lock support
 ;;;;
@@ -619,26 +615,59 @@ diff-end-of-file
 
 (defvar diff--auto-refine-data nil)
 
-;; Define diff-{hunk,file}-{prev,next}
-(easy-mmode-define-navigation
- 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))))))))))))
-
+(defun diff-navigate-maybe-refine ()
+  (when (eq diff-refine 'navigation)
+    (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))))))))))))
+
+(defun diff-hunk-next (&optional count interactive)
+  "Go to the next COUNT'th hunk.
+
+Interactively, COUNT is the prefix numeric argument, and defaults to 1.
+
+If INTERACTIVE is non-nil and `diff-refine' is set to
+`navigation', refine the hunk moved to."
+  (interactive "p\np")
+  (unless count (setq count 1))
+  (if (< count 0)
+      (diff-hunk-prev (- count))
+    (if (looking-at diff-hunk-header-re) (setq count (1+ count)))
+    (if (not (re-search-forward diff-hunk-header-re nil t count))
+        (if (looking-at diff-hunk-header-re)
+            (goto-char (diff-end-of-hunk))
+          (user-error "No next hunk"))
+      (goto-char (match-beginning 0))
+      (if interactive (diff-navigate-maybe-refine)))))
+
+(defun diff-hunk-prev (&optional count interactive)
+  "Go to the previous COUNT'th hunk.
+
+Interactively, COUNT is the prefix numeric argument, and defaults to 1.
+
+If INTERACTIVE is non-nil and `diff-refine' is set to
+`navigation', refine the hunk moved to."
+  (interactive "p\np")
+  (unless count (setq count 1))
+  (if (< count 0)
+      (diff-hunk-next (- count))
+    (unless (re-search-backward diff-hunk-header-re nil t count)
+      (user-error "No previous hunk"))
+    (if interactive (diff-navigate-maybe-refine))))
+
+;; Define diff-file-{prev,next}
 (easy-mmode-define-navigation
  diff-file diff-file-header-re "file" diff-end-of-file)
 
@@ -2125,7 +2154,7 @@ diff--refine-hunk
 
 (defun diff--font-lock-refined (max)
   "Apply hunk refinement from font-lock."
-  (when diff-font-lock-refine
+  (when (eq diff-refine 'font-lock)
     (when (get-char-property (point) 'diff--font-lock-refined)
       ;; Refinement works over a complete hunk, whereas font-lock limits itself
       ;; to highlighting smallish chunks between point..max, so we may be
diff --git a/lisp/vc/smerge-mode.el b/lisp/vc/smerge-mode.el
index 569797e..e700e32 100644
--- a/lisp/vc/smerge-mode.el
+++ b/lisp/vc/smerge-mode.el
@@ -44,7 +44,7 @@
 ;;; Code:
 
 (eval-when-compile (require 'cl-lib))
-(require 'diff-mode)                    ;For diff-auto-refine-mode.
+(require 'diff-mode)                    ;For diff-refine.
 (require 'newcomment)
 
 ;;; The real definition comes later.
@@ -264,7 +264,7 @@ font-lock-keywords
 
 ;; Define smerge-next and smerge-prev
 (easy-mmode-define-navigation smerge smerge-begin-re "conflict" nil nil
-  (if diff-auto-refine-mode
+  (if (eq diff-refine 'navigation)
       (condition-case nil (smerge-refine) (error nil))))
 
 (defconst smerge-match-names ["conflict" "upper" "base" "lower"])