Re: [PATCH 0/2] RFC: implement new zdiff3 conflict style

[Jeff King <peff@peff.net>]

On Wed, Jun 16, 2021 at 09:57:49AM +0100, Phillip Wood wrote:

> > > which seems worse. I haven't dug/thought carefully enough into your
> > > change yet to know if this is expected, or if there's a bug.
> 
> XDL_MERGE_ZEALOUS coalesces adjacent conflicts that are separated by fewer
> than four lines. Unfortunately the existing code in
> xdl_merge_two_conflicts() only coalesces 'ours' and 'theirs', not 'base'.
> Applying
> [...]
> gives
> 
> <<<<<<< HEAD
> 		if (starts_with(arg, "--informative-errors")) {
> 			informative_errors = 1;
> 			continue;
> 		}
> 		if (starts_with(arg, "--no-informative-errors")) {
> ||||||| 2f93541d88
> 		if (!prefixcmp(arg, "--informative-errors")) {
> 			informative_errors = 1;
> 			continue;
> 		}
> 		if (!prefixcmp(arg, "--no-informative-errors")) {
> =======
> 		if (!strcmp(arg, "--informative-errors")) {
> 			informative_errors = 1;
> 			continue;
> 		}
> 		if (!strcmp(arg, "--no-informative-errors")) {
> >>>>>>> 0c52457b7c^2
> 
> Which I think is correct. Whether combining single line conflicts in this
> way is useful is a different question (and is independent of your patch). I
> can see that with larger conflicts it is worth it but here we end up with
> conflicts where 60% of the lines are from the base version. One the other
> hand there are fewer conflicts to resolve - I'm not sure which I prefer.

Thanks for figuring that out. I agree that the output after the patch
you showed is correct, in the sense that the common lines show up in the
base now. It does feel like it's working against the point of zdiff3,
though, which is to reduce the number of common lines shown in the
"ours" and "theirs" hunks.

Likewise, I think this coalescing makes things worse even for "merge",
where you get:

  <<<<<<< ours
                  if (starts_with(arg, "--informative-errors")) {
                          informative_errors = 1;
                          continue;
                  }
                  if (starts_with(arg, "--no-informative-errors")) {
  =======
                  if (!strcmp(arg, "--informative-errors")) {
                          informative_errors = 1;
                          continue;
                  }
                  if (!strcmp(arg, "--no-informative-errors")) {
  >>>>>>> theirs

and have to figure out manually that those interior lines are common.
But I imagine there are cases where you have a large number of
uninteresting lines (blank lines, "}", etc) that create a lot of noise
in the output by breaking up the actual changed lines into tiny hunks
that are hard to digest on their own.

-Peff