From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mon, 15 Mar 2010 08:32:38 GMT
Subject: Re: [PATCH 04/10] checkout --conflict=diff3: add a label for ancestor
Message-ID: <20100315083238.GA10068@progeny.tock>
In-Reply-To: <7vvdcy0zf3.fsf@alter.siamese.dyndns.org>

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:

>> git rerere will not have trouble parsing this output, since instead of
>> looking for a newline, it looks for whitespace after the |||||||
>> marker.
>
> Missing:
> 
>     "... and adding the extra label will not affect the computed the conflict
>     identifier, so existing rerere database will not be invalidated with this
>     change either".
> 
> I didn't verified the above claim, but if it does not hold true, then we
> need to think the transition strategy.  I don't expect a problem, though.

It holds.  handle_path() in rerere.c actually recreates the conflict hunk
after parsing it:

	rerere_io_putconflict('<', marker_size, io);
	rerere_io_putmem(one.buf, one.len, io);
	rerere_io_putconflict('=', marker_size, io);
	rerere_io_putmem(two.buf, two.len, io);
	rerere_io_putconflict('>', marker_size, io);

This is a piece of defensive programing by Dscho that has lasted since
rerere was first made builtin.  It shelters rerere from changes in the
format of the conflict markers.

>> Reported-by: Stefan Monnier <monnier@iro.umontreal.ca>
>> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
>
> This "Reported" feels very odd for a feature enhancement ("Requested"
> would be more appropriate) not a bugfix.

Makes sense, thanks.

>> ---
>> Since “original” a good name for the common ancestor?  I also
>> considered “base” and “ancestor”; the latter is too jargon-y for my
>> taste, but “base” seems all right.
>
> Yeah, base sounds good.  Even though at the lowest level, a merge is a
> merge between two equals, people tend to think of the contents of their
> own side "original" (vs the merge outcome "result").

Sound good.  “base” it is, then.

Thanks,
Jonathan