Hi,
On Fri, 20 Jun 2008, Junio C Hamano wrote:
> diff --git a/builtin-merge-recursive.c b/builtin-merge-recursive.c
> index 4aa28a1..a355e7a 100644
> --- a/builtin-merge-recursive.c
> +++ b/builtin-merge-recursive.c
> @@ -650,9 +655,26 @@ static int merge_3way(mmbuffer_t *result_buf,
> fill_mm(a->sha1, &src1);
> fill_mm(b->sha1, &src2);
>
> + if (index_only)
> + favor = 0;
> + else {
> + switch (merge_recursive_variants) {
> + case MERGE_RECURSIVE_OURS:
> + favor = XDL_MERGE_FAVOR_OURS;
> + break;
> + case MERGE_RECURSIVE_THEIRS:
> + favor = XDL_MERGE_FAVOR_THEIRS;
> + break;
> + default:
> + favor = 0;
> + break;
> + }
Hrm. I would have preferred something like this:
if (!index_only && merge_recursive_variants == MERGE_RECURSIVE_OURS)
favor = XDL_MERGE_FAVOR_OURS;
if (!index_only && merge_recursive_variants == MERGE_RECURSIVE_THEIRS)
favor = XDL_MERGE_FAVOR_THEIRS;
else
favor = 0;
> + }
> + flag = LL_MERGE_FLAGS(index_only, favor);
> +
> merge_status = ll_merge(result_buf, a->path, &orig,
> &src1, name1, &src2, name2,
> - index_only);
> + flag);
Sorry, but in my opinion this flag mangling makes the whole code uglier.
Why not just add another parameter?
Or if you are really concerned about future enhancements to ll_merge(),
use a struct.
> @@ -1379,11 +1401,18 @@ int cmd_merge_recursive(int argc, const char **argv, const char *prefix)
> struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
> int index_fd;
>
> + merge_recursive_variants = 0;
> if (argv[0]) {
> int namelen = strlen(argv[0]);
> if (8 < namelen &&
> !strcmp(argv[0] + namelen - 8, "-subtree"))
> - subtree_merge = 1;
> + merge_recursive_variants = MERGE_RECURSIVE_SUBTREE;
> + else if (5 < namelen &&
> + !strcmp(argv[0] + namelen - 5, "-ours"))
> + merge_recursive_variants = MERGE_RECURSIVE_OURS;
> + else if (7 < namelen &&
> + !strcmp(argv[0] + namelen - 7, "-theirs"))
> + merge_recursive_variants = MERGE_RECURSIVE_THEIRS;
This just cries out loud for a new function suffixcmp().
I will not say anything about the long lines in git-merge.sh, since I
fully expect builtin-merge to happen Real Soon Now.
Anyhow, your comments about this driving the wrong workflow still apply.
Maybe we want to display them really, really prominently in
Documentation/merge-strategies.txt.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
