git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Christian Couder <chriscool@tuxfamily.org>
Date
Dec 30, 2009, 05:47 UTC
Message-ID
<200912300647.25184.chriscool@tuxfamily.org>
In-Reply-To
<7viqcbn9w3.fsf@alter.siamese.dyndns.org>
On samedi 12 décembre 2009, Junio C Hamano wrote:
Show 12 quoted lines
> Christian Couder <chriscool@tuxfamily.org> writes:
>
> >   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
[...]
Show 16 quoted lines
> 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.
Show 5 quoted lines
> 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.

Previous: Junio C HamanoNext: Christian Couder
Message 15 of 18 in “"git reset --merge" related improvements”
  1. 0/7 "git reset --merge" related improvementsChristian Couder, Dec 12, 2009
  2. 1/7 reset: do not accept a mixed reset in a .git dirChristian Couder, Dec 12, 2009
  3. Junio C HamanoDec 12, 2009
  4. Christian CouderDec 15, 2009
  5. Junio C HamanoDec 15, 2009
  6. Junio C HamanoDec 15, 2009
  7. Christian CouderDec 16, 2009
  8. Junio C HamanoDec 15, 2009
  9. 2/7 reset: add a few tests for "git reset --merge"Christian Couder, Dec 12, 2009
  10. Junio C HamanoDec 12, 2009
  11. 3/7 reset: use "unpack_trees()" directly instead of "git read-tree"Christian Couder, Dec 12, 2009
  12. Junio C HamanoDec 12, 2009
  13. 4/7 reset: add option "--keep" to "git reset"Christian Couder, Dec 12, 2009
  14. Junio C HamanoDec 12, 2009
  15. Christian CouderDec 30, 2009
  16. 5/7 reset: add test cases for "--keep" optionChristian Couder, Dec 12, 2009
  17. 6/7 Documentation: reset: describe new "--keep" optionChristian Couder, Dec 12, 2009
  18. 7/7 Documentation: reset: add some tables to describe the different optionsChristian Couder, Dec 12, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.