Re: [PATCH v5 4/7] reset: add option "--keep" to "git reset"

On samedi 12 décembre 2009, Junio C Hamano wrote:

Christian Couder [off-list ref] writes:

quoted

  B      B     C     D   --keep    (disallowed)
                         --merge    C      C     C

For --keep this is the same as the first case (except that the "partially
updated the index" happened to be "100% pertially") and it makes sense to
disallow it.

However, I think --merge _should_ have D (target) in all of the three in
the result in this case, as I mentioned in my response to [PATCH 3/7]. 
Is that "the bug" you talked about there?

No it is not the bug I talked about. It was a bug in the documentation 
patch. Thanks for finding it! I left some "C" on the merge line from an 
earlier documentation patch but I should have changed them to "D" like 
that:

   B      B     C     D   --keep    (disallowed)
                          --merge    D     D     D

[...]

It also is tempting to say that we should forbid "reset --merge" without
an unmerged entry in the index, but that wouldn't work.  A mergy
operation would have left unmerged entries in the index initially before
giving the control back to the user, but the user may have used "edit &&
git add" to resolve them, and then decided that it is not worth
committing.  By the time "reset --merge" is run, there may not be any
unmerged path left in the index.

I am suggesting extra safety measures primarily because I am worried that
people will get confused by these two similar looking options that are
meant for entirely different use cases.  An obvious alternative solution
to avoid the confusion is not to add "--keep" in the first place.  While
I think that is rather a cop-out than a solution, it might make more
sense. It is hopeless to educate users to pick the right one, if even the
author of the new option mistakenly thinks that "--keep is merely a
better version of --merge".

I just think it is better in some cases not always.

My preference is at this point to first have patches [1/7] to [3/7] to
update "reset --merge" (I am not sure about the "--mixed in $GIT_DIR"
change, though), with a new follow-up patch [3.5/7] to fix "reset
--merge" to reset paths that were cloberred by the merge to the target
(not HEAD), and start cooking these changes in 'pu' and then 'next'.

Ok, I will send a patch series to do that except that I I don't understand 
exactly what the follow up patch [3.5/7] shoud do. Perhaps you asked for 
this additional patch because of the documentation bug above?

That will lay groundwork of using unpack_trees() in "reset" and after
they stabilize, build new modes like "--keep" on top of it.

Ok.

Thanks,
Christian.