Add option to auto recenter after diff-hl-{next,previous}-hunk

[NicholasBHubbard]

Hello. This PR adds a new variable diff-hl-next-previous-hunk-auto-recenter, that when set causes diff-hl-next-hunk and diff-hl-previous-hunk to automatically call recenter after moving the point.

In my configuration I use after advice on these functions to do this auto-recentering, but I thought it would be nicer if there was a diff-hl variable for this behavior.

Let me know if you think this is a good idea, and if there is anything you would like changed in this PR. Thanks!

[dgutov]

Hi!

Could this approach be a little problematic?

This function is also called from diff-hl-stage-current-hunk and diff-hl-revert-hunk, where I'm not sure if recentering is good or not.

[NicholasBHubbard]

Thanks for the response!

I just looked and found that indeed diff-hl-stage-current-hunk and diff-hl-revert-hunk do call diff-hl-next-hunk indirectly through a call to diff-hl-find-current-hunk. It never occurred to me that you didn't need to have point on a hunk in order to call diff-hl-stage-current-hunk or diff-hl-revert-hunk, as I personally only ever call these functions when I have point on a hunk. This is a nice feature! I agree with you that the new behavior from this PR is problematic for this reason.

I came up with a simple solution to just disable diff-hl-next-previous-hunk-auto-recenter from diff-hl-find-current-hunk. See commit 3ffc55c.

[NicholasBHubbard]

Alternate solution to 3ffc55c:

diff --git a/diff-hl.el b/diff-hl.el
index 6b2fa14..0081e5f 100644
--- a/diff-hl.el
+++ b/diff-hl.el
@@ -1082,14 +1082,14 @@ its end position."
           (when (and o (= (overlay-start o) (point)))
             (throw 'found o)))))))
 
-(defun diff-hl-next-hunk (&optional backward)
+(defun diff-hl-next-hunk (&optional backward no-recenter)
   "Go to the beginning of the next hunk in the current buffer."
   (interactive)
   (let ((overlay (diff-hl-search-next-hunk backward)))
     (unless overlay
       (user-error "No further hunks found"))
     (goto-char (overlay-start overlay))
-    (when diff-hl-next-previous-hunk-auto-recenter
+    (when (and diff-hl-next-previous-hunk-auto-recenter (not no-recenter))
       (recenter))))
 
 (defun diff-hl-previous-hunk ()
@@ -1104,7 +1104,7 @@ its end position."
      ((setq o (diff-hl-search-next-hunk t))
       (goto-char (overlay-start o)))
      (t
-      (diff-hl-next-hunk)))))
+      (diff-hl-next-hunk nil t)))))
 
 (defun diff-hl-mark-hunk ()
   (interactive)

EDIT: Would also want to update diff-hl-previous-hunk to take an optional no-recenter arg that is passed along to diff-hl-next-hunk

[NicholasBHubbard]

Any update on this? Do others think this would be a good addition to diff-hl?

[frou]

Do others think this would be a good addition to diff-hl?

Yeah - I think it's a good idea

[dgutov]

Thanks for the reminder and the feedback! Merging 🚀