Re: [RFC/PATCH v2] merge-base: teach "git merge-base" to accept more than 2 arguments
- From
Johannes Schindelin <johannes.schindelin@gmx.de>
- Date
- Jul 27, 2008, 14:38 UTC
- Message-ID
- <alpine.DEB.1.00.0807271631470.5526@eeepc-johanness>
- In-Reply-To
- <20080727053324.b54fe48e.chriscool@tuxfamily.org>
Hi,
On Sun, 27 Jul 2008, Christian Couder wrote:
Show 22 quoted lines
> diff --git a/builtin-merge-base.c b/builtin-merge-base.c
> index 1cb2925..f2c9756 100644
> --- a/builtin-merge-base.c
> +++ b/builtin-merge-base.c
> @@ -38,15 +48,22 @@ int cmd_merge_base(int argc, const char **argv, const char *prefix)
> usage(merge_base_usage);
> argc--; argv++;
> }
> - if (argc != 3)
> + if (argc < 3)
> usage(merge_base_usage);
> - if (get_sha1(argv[1], rev1key))
> - die("Not a valid object name %s", argv[1]);
> - if (get_sha1(argv[2], rev2key))
> - die("Not a valid object name %s", argv[2]);
> - rev1 = lookup_commit_reference(rev1key);
> - rev2 = lookup_commit_reference(rev2key);
> - if (!rev1 || !rev2)
> +
> + rev1 = get_commit_reference(argv[1]);
> + if (!rev1)
> return 1;Why do you special case rev1? Is it so special? Just handle it together with all of the other arguments!
IOW have one commit array, and do not call it "prev".
Show 11 quoted lines
> - return show_merge_base(rev1, rev2, show_all);
> + argc--; argv++;
> +
> + do {
> + struct commit *rev2 = get_commit_reference(argv[1]);
> + if (!rev2)
> + return 1;
> + ALLOC_GROW(prev2, prev2_nr + 1, prev2_alloc);
> + prev2[prev2_nr++] = rev2;
> + argc--; argv++;
> + } while (argc > 1);Now, this is ugly. You know beforehand the _exact_ number of arguments, and yet you dynamically grow the array? Also, why do you use a do { } while(), when a for () would be much, much clearer?
BTW I seem to recall that get_merge_bases_many() was _not_ the same as get_merge_octopus(). Could you please remind me what _many() does?
Ciao, Dscho