[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
bug#32991: 27.0.50; diff-auto-refine-mode a no-op
From: |
Charles A. Roelli |
Subject: |
bug#32991: 27.0.50; diff-auto-refine-mode a no-op |
Date: |
Mon, 18 Feb 2019 20:06:35 +0100 |
> From: Stefan Monnier <monnier@IRO.UMontreal.CA>
> Date: Mon, 11 Feb 2019 15:14:12 -0500
>
> >> This change seems unrelated. I'd rather we stick to the refinement itself.
> > Without this change, other functions in diff-mode (such as
> > diff--font-lock-syntax) calling diff-hunk-next accidentally trigger
> > hunk refinement if 'diff-refine' is 'navigation'.
>
> Ah, right, makes sense. Could we fix this more directly by using
> `called-interactively` instead?
I think so, though I avoided that function because of the warnings in
its documentation. Future callers of diff-hunk-prev/diff-hunk-next
also have more flexibility if they can choose whether the call should
be considered interactive or not.
> Indeed, I see several changes in there, which is why I'd rather we
> separate them into another patch.
>
> [ FWIW I never used the restriction (I coded it up only to mach the
> featureset of some earlier diff-mode I'd found somewhere), but I would
> miss the recentering. More to the point, rather than removing the
> recentering, I'd like to improve it (so it tries harder to make the
> whole hunk visible when possible). ]
For a bit of background: I sometimes enable a minor mode in the
ChangeLog buffer which narrows *vc-diff* (if displayed) to the changes
of the file at point, like an rmail summary buffer with its
corresponding message buffer. I'd like to eventually add this feature
to add-log.el. The current diff-hunk-prev/diff-hunk-next change the
narrowing of the buffer and thus cause some friction with this minor
mode.
That said, I could still add this new feature if I could disable the
re-narrowing by, say, setting a buffer-local variable. I will update
the patch to bring back the re-narrowing by default, and then add a
defcustom for it in a second patch (ditto for the re-centering).
> Other than that, LGTM.
Thanks for the review.