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

On Wed, Jun 23, 2021 at 10:53:25AM +0100, Phillip Wood wrote:

quoted

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.

I agree - the output is longer rather than shorter. As we only want to trim
the common prefix and suffix from the conflicts I wonder if it would be
better to special case zdiff3 rather than piggy backing on the existing
XDL_MERGE_ZEALOUS implementation. We can trim the common lines by looping
over the begging and end of the hunk comparing the lines with xdl_recmatch()
without going to the trouble of diffing them as XDL_MERGE_ZEALOUS does. I
don't think we need to worry about coalescing adjacent conflicts for zdiff3.
It makes sense to coalesce in the XDL_MERGE_ZEALOUS case as it can
potentially split a  N line conflict hunk into N/2 single line conflict
hunks but zdiff3 does not split conflict hunks.

That matches my intuition of a reasonable path forward (but I confess to
not being too well-versed in the details of the XDL_MERGE internals).

Yes, I think the heuristic for coalescing conflict hunks could be improved.
It would be fairly simple to only join two hunks if the conflicts are longer
that the context between them and the existing XDL_MERGE_ZEALOUS_ALNUM logic
allows conflicts with more context between them to be coalesced if the
context lines are uninteresting. I think XDL_MERGE_ZEALOUS_ALNUM is only
used by `git merge-file` at the moment, with everything else going through
ll_merge() which uses XDL_MERGE_ZEALOUS

I don't recall much discussion around using ALNUM versus not, nor could
I find much in the archive. It looks like merge-file was converted to
show off ALNUM when it was added in ee95ec5d58 (xdl_merge(): introduce
XDL_MERGE_ZEALOUS_ALNUM, 2008-02-17), and it never really progressed
from there.

It might be an interesting exercise to re-run a bunch of merges and see
if ALNUM produces better (or worse) results on the whole.

-Peff