Re: [PATCH 2/2 - RFH/WIP] xdiff-merge: optionally show conflicts in "diff3 -m" style

Junio C Hamano <gitster@pobox.com> writes:

I've been using this for my own work, and here is a typical example. This happened while rebuilding 'pu', merging jc/merge-whitespace topic on top of what contains jc/better-conflict-resolution topic.

    In case if anybody is interested in reproducing this to follow the
    discussion of improving the support of conflicted merge resolution,
    here is how to reproduce this:

        $ git checkout 7ead6a7^
        $ git merge ^2

In builtin-merge-file.c, there appears:

	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
<<<<<<< HEAD:builtin-merge-file.c
			&xpp, merge_level | merge_style, &result);
|||||||
			&xpp, XDL_MERGE_ZEALOUS_ALNUM, &result);
=======
			&xpp, XDL_MERGE_ZEALOUS_ALNUM, ws_rule, &result);

	for (i = 0; i < 3; i++)

In this case, both branches are my code, so I do not need the additional hint to see that one branch added ws_rule parameter while the other one generalized the "merge simplify level" parameter and is now passing a bitwise OR of two variables. But if somebody else is doing the merge, it may not be so obvious how the resolution should be.

On the other hand, this appears in ll-merge.c:

	xpparam_t xpp;
<<<<<<< HEAD:ll-merge.c
	int style = 0;
|||||||
=======
	unsigned ws_rule = 0;

This is obviously useless; perhaps we can drop the "original" hunk when it is empty to save space?

Another thing that helped me figure out what was going on was when merging mv/merge-recursive topic on top of next.

    In case if anybody is interested in reproducing this to follow the
    discussion of improving the support of conflicted merge resolution,
    here is how to reproduce this:

        $ git checkout ebe0e3b^
        $ git merge ebe0e3b^2

This merge involves removal of quite a lot of lines to a new file merge-recursive.c from existing builtin-merge-recursive.c; the default conflict simplification (XDL_MERGE_ZEALOUS) works _extremely_ well for this merge, in that it shows this (I'll refer to this result as *1* in a later paragraph):

    <<<<<<<
    static struct commit *get_ref(const char *ref)
    {
    ...
    }

    static int merge_config(const char *var, const char *value, void *cb)
    {
    ...
    }

    =======
    >>>>>>>

It just shows we have _some_ changes to these two functions since the other side removed them from this file, but it does not say what the change is. However, with the "common ancestor" version, here is what I get (you need to use 'next' and "merge.conflictstyle" configuration set to "diff3" to reproduce this):

    <<<<<<<
    static struct commit *get_ref(const char *ref)
    {
    ...
    }

    static int merge_config(const char *var, const char *value, void *cb)
    {
    ...
            return git_xmerge_config(var, value, cb);
    }

    |||||||
    static struct commit *get_ref(const char *ref)
    {
    ...
    }

    static int merge_config(const char *var, const char *value, void *cb)
    {
    ...
            return git_default_config(var, value, cb);
    }

    =======
    >>>>>>> theirs

By comparing the first and second part, we can tell that the call to git_default_config() was replaced to a call to git_xmerge_config(). And this is a crucial clue to me to make an corresponding update to the function that now lives in merge-file.c.

On average, I am finding that "diff3 -m" format more irritating than useful. However, on occasions like this, I am finding it quite useful.

My observation so far suggests that it would be best for me to leave the configuration "merge.conflictstyle" to the default "merge", and instead give an option to allow me to tell "git checkout -m -- $path" (which is also a new feature; it overwrites the $path by the result of a fresh merge to reproduce the conflicted state in the working tree, using the three stages recorded in the index) to use "diff3 -m" style, when I want to.

That means the command sequence would look like:

    $ git merge mv/merge-recursive ;# git merge ebe0e3b^2
    .. oops, heavily conflicted merge and I see *1*, which is not
    .. very useful.  Let's see what is going on.
    .. Note that this --merge=diff3 option does not exist yet.
    $ git checkout --merge=diff3 -- builtin-merge-recursive.c
    $ emacs builtin-merge-recursive.c
    .. split the buffer into two, move the cursor to the line after <<<
    .. in one window while moving the cursor to the line after ||| line
    .. in the other one, and say "M-x compare-windows".  Aha, there is
    .. a change there.

Having said all that, I now realize that the way I described in my response to you is just as easy to view this particular case:

    $ git merge mv/merge-recursive ;# git merge ebe0e3b^2
    .. oops, heavily conflicted merge and I see *1*, which is not
    .. very useful.  Let's see what is going on.
    .. Note that this --merge=diff3 option does not exist yet.
    $ git diff :{1,2}:builtin-merge-recursive.c ;# shell expands {1,2} ;-)

which exactly shows what one side changed -- instead of calling git_default_config(), the code calls git_xmerge_config().

$ git diff :{1,2}:builtin-merge-recursive.c