Subject: Re: Merge-Recursive Improvements

From: Junio C Hamano <[email-masked]>
Date: Tue, 12 Feb 2008 15:11:19 -0800

Voltage Spike <[email-masked]> writes:

> I would like to make a series of significant improvements to the
> merge-recursive mechanism in git, but I was hoping to solicit some early
> feedback before submitting patches.
>
> First, git is overly zealous at merging differences and two functions
> added
> at the same point in a file become intertwined during the merge. A
> trivial
> example of this behavior:
>
>   <<<<<<< HEAD:file.txt
>   void newfunc1()
>   =======
>   void newfunc2()
>   >>>>>>> merge:file.txt
>   {
>     int err;
>   <<<<<<< HEAD:file.txt
>     err = doSomething();
>   =======
>     err = doSomethingElse();
>   >>>>>>> merge:file.txt

This lacks illustration of what you change that example to, which
makes the proposal harder to judge.

I suspect you are saying that you would want to coalesce
adjacent hunks that have too small number of lines between '>>>'
of the previous hunk and '<<<' of the current hunk by duplicate
the common hunks, like this:

   <<<<<<< HEAD:file.txt
   void newfunc1()
   {
     int err;
     err = doSomething();
   =======
   void newfunc2()
   {
     int err;
     err = doSomethingElse();
   >>>>>>> merge:file.txt

(here, two lines that are "{" and "int err;" are taken as "too small").

I think it makes sense.

> Second, git doesn't tell me the original code inside the conflict
>
>   >>>>>>> merge-base:file.txt
>   Original code.
>   ======= HEAD:file.txt
>   Head code.
>   ======= merge:file.txt
>   Merged code.
>   <<<<<<<

This is a much harder sell, as external tool like git-mergetool
that inspect the result depend on the current output.

And it is not as useful as an alternative.

In case you did not know, you can get a much better picture by:

    $ git log --left-right -p --merge

because you would then see not just the merge base version but
the changes _and the reasons for the changes_ in between.

> Third, git doesn't appear to have any sense of context when performing a
> merge. Another contrived example which wouldn't be flagged as a merge
> conflict:
>
>   ptr = malloc(len); // Added in HEAD.
>   init();            // Included in merge-base.
>   ptr = malloc(len); // Added in "merge".

Are you saying it a problem to report or not to report?  In
either case, I decline to comment on this one, as I do not have
a strong opinion either way.

> Fourth, git doesn't provide a mechanism for merges to ignore whitespace
> changes.

That would be a good change.

I can immediately say that 1 and 4 are worthwhile things to do,
as long as they are contained to xdl_merge().  It would help
other users of the merge logic.

I've started working on rewriting revert to directly use
xdl_merge(), bypassing major parts of merge-recursive, and I
imagine such a change you propose would be useful without
affecting the callers.