[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#25105: 26.0.50; diff navigation is broken

From: Tino Calancha
Subject: bug#25105: 26.0.50; diff navigation is broken
Date: Sat, 7 Jan 2017 20:16:45 +0900 (JST)
User-agent: Alpine 2.20 (DEB 67 2015-01-07)

On Sat, 7 Jan 2017, Dima Kogan wrote:

Dear Dima,
i appreciate very much your efforts to improve diff-mode.
In particular, i like 1. and 2. in your commit message.
That are useful goals.  I just disagree with the implementation.
In summary, i believe a better approach is to get 1., 2.
without affecting the behaviour of `n', `p'.

   1. It should be possible to place the point in the middle of a diff
   buffer, and press M-k repeatedly to kill hunks in the order they appear
   in the buffer.  With the point on hunk1, M-k M-k would kill hunk1 then
   hunk2.  With the point on hunk3, it would kill hunk3 then hunk4; this is
   fine.  However, with the point on hunk2, it'd kill hunk2 then hunk1.
   This is fixed by this patch.

   2. Similarly, it should be possible to apply hunks in order.  Previously
   with the point at the start, C-c C-a would apply the hunk1, then move
   the point to the first @@ header, and thus C-c C-a would try to apply
   the same hunk again.
Your patch is invasive: it should be possible to get 1) an 2) above without
redefining a long standing behaviour for `diff-hunk-next' and `diff-hunk-prev'.
In addition, that change in `n' and `p' would deserve a more prominent 
in the commit message, f.i., a new dot 3., so people would be aware of
this subtle change.

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"
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 
`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
;; 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.

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).

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!

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]