bug#25105: 26.0.50; diff navigation is broken

[Dima Kogan]

Tino Calancha <tino.calancha@gmail.com> writes:

> On Sat, 7 Jan 2017, Dima Kogan wrote:
>
>>Let's look at the commands required to apply hunks 1 and 2 in a buffer
>>versus just hunk 2. Assuming we're at bob, and assuming we're using the
>>new code.
>>
>>  Applying hunks 1 and 2:  C-c C-a   C-c C-a; i.e. apply 1, apply 2
>>  Applying hunk 2 only:    M-n       C-c C-a; i.e. skip 1,  apply 2
>>
>>If M-n works the way it did before, then you need to invoke M-n twice to
>>apply hunk 2 only. I claim this is weird, since it should require only
>>one command to "skip 1". This is what is meant by a "consistent"
>>behavior.
> Well, not everyone follow that logic.  I like to read carefully the hunks
> of a patch _before_ decide to apply then.  I can do that with easy using `n' 
> and `p'
> keys.  If the commit message is long, as in your commit 2c8a7e5, then an user 
> need
> `n' to jump to the first hunk and read it; but after your patch the first hunk
> is skipped.  Then the user need to be a believer and apply it without seeing
> it using `C-c C-a'; non believer users, might prefer to rewind and read the 
> hunk
> as follows:
> M-! git show 2c8a7e5 RET
> C-o C-x C-q
> M-x diff-mode RET
> n
> ;; we are at 2nd hunk, i.e., at line:
> @@ -570,7 +570,102 @@ diff--auto-refine-data
> ;; Assuming no split window, so we can see clearly the first hunk;
> ;; after looking at it carefully, we decided is good, so let's apply it:
> p ; jump to 1st hunk: this is _extra_ compared with Emacs-25!
> C-c C-a
>
> So, it might be argued that this patch force users to gratuitously push
> _always_ an extra `p' to read/apply the first hunk of _every_ patch.
> That makes hunk navigation much less pleasant.

This clearly shows that it would be useful to have some binding that
jumps to the beginning of the current hunk. But that's not the same as
moving to the 'next' hunk, which is what 'n' ostensibly is supposed to
do.


>>Furthermore, I'd like to get some feedback from more than just the few
>>people active on this thread. If there's a lopsided preference to the
>>old approach, we can simply revert and move on.
> As a rule of thumb, if i get N people here uncomfortable with one of my
> patches on master branch, i believe that can easily be translated
> to (* 100 N) people once the patch is in a stable release (at least).

Right. The choice is whether to revert the new behavior entirely, or to
leave it in with a switch. Since we're looking at a small sample here,
it's not clear which is the right move.


>>I'm fine with a switch that lets you pick what you
>>want. As stated above, I don't think a hybrid approach makes sense,
>>since it takes one weird thing and converts it to another thing that's
>>weird in a different way.
> FWIW, I am working in an alternative patch: it would satisfy your 1. and 2.
> while respecting `n' and `p'.  Stay tune!

Great!

dima