bug#25105: 26.0.50; diff navigation is broken

[Dima Kogan]

Mark Oteiza <mvoteiza@udel.edu> writes:

> On 05/01/17 at 05:27pm, Dima Kogan wrote:
>> Dima Kogan <dima@secretsauce.net> writes:
>>
>> > The behavior I want is to always have a consistent idea of which hunk we
>> > are currently on.
>>
>> Some more behaviors that I think are desirable are described in the
>> commit message of the main patch:
>>
>>   https://github.com/emacs-mirror/emacs/commit/2c8a7e50d24daf19e
>
> The only mention of the changes to navigation is "Better navigation
> logic".  Not documented in NEWS, and no tests for the corner cases.

I just re-read the commit message, and I think it's clear that it
touches navigation. I'm not a seasoned contributor to this project, so I
don't know what the policy is regarding NEWS. In either case, that is
irrelevant: we'd still be having this conversation.

Tests would be good, as always. However the diff-mode prior to this
patch had no tests, and I've been using this code every day for years.
And again, this doesn't matter. The issues in question aren't unintended
bugs, and they would have passed any tests I would have written.



> I fail to see how fixing corner cases in diff-apply-hunk has anything
> to do with diff-{file,hunk}-{next-prev}

The issues being fixed are making anything that operates on hunks more
consistent, so diff-{file,hunk}-{next-prev} are relevant.


> At first glance, it looks like the following patch would restore the
> previous behavior, however it completely breaks auto refinement.
>
> <snip>

If you want to restore the previous behavior, wouldn't a revert be
better? Or are you trying to restore only a subset of the previous
behavior?


> With the number of actual bugs (email/format-patch/pre-diff content,
> and auto refinement) the initial patch caused, perhaps this is best.

The email/format-patch issue has nothing to do with me; it has been a
problem for years. The way to "fix" auto-refinement is to invoke
auto-refinement in a diff-mode-hook, as suggested earlier. The bug
reporter didn't like that, and I don't know what they want. I'm not sure
where the pre-diff content issue came from. Likely it came up because
the patch that was in the BTS for years wasn't what ended up being
merged, so I haven't sufficiently tested it. Lesson learned.

I consider the current behavior a significant improvement in usability,
but if there's a consensus that it's a step backward, then I'll go back
to carrying this patch in my local tree. Let me ask the few people I
know who would be using this code at all to get at least anecdotal
feedback.