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

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 26, 2009, 06:17 UTC
Message-ID: <7vy6ltdd2l.fsf@alter.siamese.dyndns.org>
In-Reply-To: <d243a513ffb8da4272f7a0e13a711f9b65195c25.1259201377.git.apenwarr@gmail.com>

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

Show 6 quoted lines

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

Show 12 quoted lines

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