Re: [RFC/PATCH 1/4] Add git-sequencer shell prototype

[Junio C Hamano <gitster@pobox.com>]

Stephan Beyer <s-beyer@gmx.net> writes:

> +		sed -e 's/[$"\\]/\\&/g' -n -e '
>  			s/^Author: \(.*\)$/GIT_AUTHOR_NAME="\1"/p;
>  			s/^Email: \(.*\)$/GIT_AUTHOR_EMAIL="\1"/p;
>  			s/^Date: \(.*\)$/GIT_AUTHOR_DATE="\1"/p
> ###
>
> Is escaping $, " and \ enough?

Look at how it is done in git-sh-setup get_author_ident_from_commit.

>> > +	if test -n "$failed"
>> > +	then
>> > +		# XXX: This is just a stupid hack:
>> > +		with_author git apply $apply_opts --reject --index "$PATCH"
>> 
>> Please don't do this without being asked, if you are planning to use this
>> in "am" when 3-way fallback was not asked.  It _may_ make sense to give an
>> option to the users to ask for .rej if they prefer to work that way better
>> than working with 3-way merge fallback, but doing this without being asked
>> is not acceptable.
>
> The --reject was just a mind marker for that what I actually think is
> useful and less annoying than the current behavior:
>
>> > +		die_to_continue 'Patch failed. See the .rej files.'
>> > +		# XXX: We actually needed a git-apply flag that creates
>> > +		# conflict markers and sets the DIFF_STATUS_UNMERGED flag.
>> 
>> That is what -3way is all about, and this codepath is when the user did
>> not ask for it, isn't it?
>
> Now imagine you apply a patch that cannot be applied 100% cleanly and
> you don't have the 3-way base in the repo. You know what happens?

Do you think I don't?  You can check who invented 3way by running "git
log" or "git blame" on git-am.sh ;-)

I think you misread my "That is what -3way is all about".  That remark is
about the comment you have about "creates conflict markers".  The conflict
markers is only possible because we do 3-way merge when you ran "am -3".
If you do not have the base object but only a blob and an unapplicable
patch, you cannot do "here is our change since common ancestor, and here
is their change the patch wants to make" conflict markers, because you do
not have the common ancestor.

> Yes, the patch is completly rejected, because apply is atomic.
> And I think a git-apply option that results in a non-atomic behavior,
> that creates conflict markers (and no .rej files), would be a great
> usability feature for the "patch" insn in sequencer.

Yes, I think I already said in the message you are responding to that it
may make sense to have such an option (but at the same time we should
remember that nobody asked to add --reject to "git am").