Re: [PATCH v4 0/5] {checkout,reset,stash} --patch

[Thomas Rast <trast@student.ethz.ch>]

[-- Attachment #1: Type: Text/Plain, Size: 1833 bytes --]

Jeff King wrote:
> I finally got a few minutes to look at this. I tried "checkout --patch"
> first, which was very confusing:
> 
>   $ echo old content >file && git add file && git commit -m old
>   $ echo new content >>file
>   $ git checkout --patch file
>   diff --git a/file b/file
>   index 33194a0..805c3b0 100644
>   --- a/file
>   +++ b/file
>   @@ -1 +1,2 @@
>    old content
>    +new content
>   Check out this hunk [y,n,q,a,d,/,e,?]?
> 
> Shouldn't the diff be reversed? That is, I think what users would like
> to see is "bring this hunk over from the index to the working tree". But
> we have the opposite (a hunk that is in the working tree that we would
> like to undo).

Well, my thinking for the initial (restricted; you couldn't say 'git
checkout -p HEAD~14') version went something like this: 'reset -p'
should be the opposite of 'add -p', so it offers the same hunks with
the question "Reset?".  Then 'checkout -p' should somehow follow suit,
but asked "Discard?" (IIRC I even had it in all caps).

I'm not 100% happy with your suggested forward patches strategy
because I think (particularly for people with colors enabled, and I
suspect we all have), it's less confusing "my" way if they go through
'add -p' and suddenly think "oops, mistake, I need to reset that": it
is much easier for the (at least for my) eye to find the same hunk
again if it is really 100% the same.  Probably we would have to change
the verb to "discard" again, though.

OTOH this does become rather weird once you specify anything other
than HEAD.  And while we could of course switch the approach if the
user does that, and hope that he'll understand on the grounds that
it's an advanced usage, I'm not sure switching is a good idea in
general.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 197 bytes --]