Re: [PATCH 1/8] git-merge-file --ours, --theirs

[Junio C Hamano <gitster@pobox.com>]

"Avery Pennarun" <apenwarr@gmail.com> writes:

> ...
> A larger problem is that this tends to encourage a bad workflow by
> allowing them to record such a mixed up half-merge result as a full commit
> without auditing.  This commit does not tackle this latter issue.  In git,
> we usually give long enough rope to users with strange wishes as long as
> the risky features is not on by default.

Typo/Grammo.  "risky features are not on by default".

> (Patch originally by Junio Hamano <gitster@pobox.com>.)
>
> Signed-off-by: Avery Pennarun <apenwarr@gmail.com>

Except for parse-optification, this one is more or less a verbatim copy of
my patch, and I think I probably deserve an in-body "From: " line for this
[PATCH 1/8], [PATCH 6/8] and [PATCH 8/8] to take the full authorship of
them.

> diff --git a/xdiff/xdiff.h b/xdiff/xdiff.h
> index 4da052a..2cce49d 100644
> --- a/xdiff/xdiff.h
> +++ b/xdiff/xdiff.h
> @@ -58,6 +58,11 @@ extern "C" {
>  #define XDL_MERGE_ZEALOUS_ALNUM 3
>  #define XDL_MERGE_LEVEL_MASK 0x0f
>  
> +/* merge favor modes */
> +#define XDL_MERGE_FAVOR_OURS 0x0010
> +#define XDL_MERGE_FAVOR_THEIRS 0x0020
> +#define XDL_MERGE_FAVOR(flags) (((flags)>>4) & 3)

This is a bad change.  It forces the high-level layer of the resulting
code to be aware that the favor bits are shifted by 4 and it is different
from what the low-level layer expects.  If I were porting it to
parse-options, I would have kept OURS = 1 and THEIRS = 2 as the original
patch, and instead did something like:

 	ret = xdl_merge(mmfs + 1, mmfs + 0, names[0], mmfs + 2, names[2],
-			&xpp, merge_level | merge_style, &result);
+			&xpp, XDL_MERGE_FLAGS(merge_level, merge_style, merge_favor), &result);

with an updated definition like this:

    #define XDL_MERGE_FLAGS(level, style, favor) ((level)|(style)|((favor)<<4)