Re: [PATCH v5 1/1] mergetool: add automerge configuration

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

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Junio C Hamano wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>> 
>> > +auto_merge () {
>> > +	git merge-file --diff3 --marker-size=7 -q -p "$LOCAL" "$BASE" "$REMOTE" >"$DIFF3"
>> > +	if test -s "$DIFF3"
>> > +	then
>> 
>> We do not want to ignore the exit status from the command.  IOW, I
>> think the above wants to be rather
>> 
>> 	if git merge-file ... >"$DIFF3" &&
>> 	   test -s "$DIFF3"
>> 	then
>> 		...
>
> That doesn't work.
>
> "git merge-file" always returns non-zero status when it succeeds (it's
> the number of conflicts generated).

Ah, I forgot about that one.  I think "the number of conflicts" was
a UI mistake (the original that it mimics is "merge" from RCS suite,
which uses 1 and 2 for "conflicts" and "trouble") but we know we
will get conflicts, so it is wrong to expect success from the
command.  Deliberately ignoring the return status is the right thing
to do.

> What if the original file does have these markers?
>
> Which is probably something we should be checking beforehand and not
> attempt an automerge in those cases.

Yes, that is a much better approach to avoid unnecessary work.

When we made the conflict marker length configurable, we were hoping
that we no longer have to worry about the cases where payload files
(original or ours or theirs) have lines that are confusingly similar
to the conflict markers, but because we are interfacing external tools
that are unaware of the facility, it probably would not help us in
this case all that much.

FWIW, we use a fiarly large size for our own files in t/ and
Documentation/ directories ourselves, and it does help topic branch
merges somewhat frequently.