From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Alan Mackenzie <acm@muc.de>,
Dennis Kaarsemaker <dennis@kaarsemaker.net>,
git@vger.kernel.org
Subject: Re: "git stash pop" is doing an unwanted "git add" when there are conflicts.
Date: Tue, 29 Dec 2015 11:04:20 -0800 [thread overview]
Message-ID: <xmqq37ulhya3.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <20151229075329.GA9254@sigill.intra.peff.net> (Jeff King's message of "Tue, 29 Dec 2015 02:53:30 -0500")
Jeff King <peff@peff.net> writes:
> Yeah, I think I agree. But keep in mind that we have to mention the
> conflicts _somewhere_, so we're going to touch the index regardless (and
> the user is going to have to erase the conflicts in the index
> eventually, either with `git add` or `git reset`).
I do not think there is much we can do at this point in time (but
notice that I did say "much", and didn't say "anything").
The way we replay any potentially conflicting change and present the
result to the end user in Git is uniformly to add the cleanly
applied parts to the index while leaving the conflicted ones as
unmerged index entries, but "git stash apply/pop" does not follow
this pattern. I think this is partly because the command is also
used to "stash away" a work-in-progress changes to the index, and
avoiding to touch the index with the changes in the working tree
would more closely match the behaviour of the command when there is
no conflict.
I notice that I said the same thing long time ago, i.e. in the
initial round of review:
http://article.gmane.org/gmane.comp.version-control.git/50749
| However, I think it is a bit counterintuitive to update the
| working tree change to the index if the merge did not conflict.
| It might be better to run an equivalent of "git reset", so that
| after "git save restore", the user can say "git diff" (not "git
| diff HEAD") to view the local changes. Perhaps...
In the above, I suggested to "git reset" when there is no conflict.
I think this line of thinking can be followed even further to
selectively reset the paths that were cleanly merged (which is added
by the call to merge-recursive), leaving _only_ the conflicted paths.
Would that give us a better outcome? I dunno.
>> Are there any prospects of this getting fixed?
>
> Somebody needs to write a patch. I am not 100% convinced that it
> _should_ be fixed, but I am leaning that way. But I am not planning to
> work on it myself anytime soon. The best way to get more discussion
> going is to post a patch. :)
next prev parent reply other threads:[~2015-12-29 19:04 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-12-21 14:29 "git stash pop" is doing an unwanted "git add" when there are conflicts Alan Mackenzie
2015-12-21 20:34 ` Alan Mackenzie
2015-12-22 8:17 ` Dennis Kaarsemaker
2015-12-22 9:30 ` Jeff King
2015-12-24 9:20 ` Alan Mackenzie
2015-12-29 7:53 ` Jeff King
2015-12-29 19:04 ` Junio C Hamano [this message]
2015-12-30 7:13 ` Jeff King
2015-12-29 21:20 ` Alan Mackenzie
2015-12-30 7:02 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqq37ulhya3.fsf@gitster.mtv.corp.google.com \
--to=gitster@pobox.com \
--cc=acm@muc.de \
--cc=dennis@kaarsemaker.net \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.