From: Jonathan Nieder Date: Mon, 08 Mar 2010 04:54:54 GMT Subject: Re: git-core: conflictstyle=diff3 doesn't actually use diff3 compatible format Message-ID: <20100308045454.GA22672@progeny.tock> In-Reply-To: <7vpr3itn89.fsf@alter.siamese.dyndns.org> Junio C Hamano wrote: > Jonathan Nieder writes: >> I’d be interested to hear from xdiff people >> whether it should be easy to add the ancestor name to the output. > > I don't think there was any xdiff people involved in this area. > > I suspect that our tools actually rely on the common ancestor markers not > having any extra cruft after them, so it would be introducing a bug if you > change this output without changing the places that read them (I know > about "rerere", but there may be others). I guess the relevant xdiff person was you. ;-) Thank you for the quick response. On the xdiff level, it looks like all that is needed is to pass the ancestor label as a member of struct s_xmparam, and then fill_conflict_hunk() could respect that. Not complicated at all. For merge_trees() users, the ancestor label could be passed with branch1 and branch2 in struct merge_options. That leaves the question of merge_recursive(). With merge_recursive(), there is more than one ancestor, so it is not completely clear what the diff3 merge should do. Currently it writes something like this: <<<<<<< HEAD Conflict resolution is hard; let's go shopping. ||||||| <<<<<<< Temporary merge branch 1 Who knows whose this is? ||||||| Ancient history. ======= Another intermediate result. >>>>>>> Temporary merge branch 2 ======= Git makes conflict resolution easy. >>>>>>> topic which is hard to read [1]. Probably it would be better to use a consolidated common ancestor, by cocatenating the internal common ancestors; in this simple case, that would look like this: <<<<<<< HEAD Conflict resolution is hard; let's go shopping. |||||||| Ancient history. ======== Git makes conflict resolution easy. >>>>>>>> topic What should be the label of this possibly fictional merge base? Jonathan [1] For people and for rerere. See http://bugs.debian.org/569645