Re: Merge-Recursive Improvements

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 12, 2008, 23:11 UTC
Message-ID: <7vodal22js.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <A21B3CA8-6240-434F-87A9-C6F76DA15265@gmail.com>

Voltage Spike <voltspike@gmail.com> writes:

Show 22 quoted lines

> 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.

Show 9 quoted lines

> 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.

Show 7 quoted lines

> 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.