Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options

From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Dec 8, 2011, 16:47 UTC
Message-ID: <CALkWK0mmjKSzSbxq2i7=JvcB4LTro-MYDCwQLUUwqcf8qS0zPA@mail.gmail.com>
In-Reply-To: <20111208163946.GB2394@elie.hsd1.il.comcast.net>

Hi,

Jonathan Nieder wrote:

Show 24 quoted lines

> Ramkumar Ramachandra wrote:
>
>> -     if (argc < 3)
>> -             usage(builtin_bundle_usage);
>> -
>> -     cmd = argv[1];
>> -     bundle_file = argv[2];
>> -     argc -= 2;
>> -     argv += 2;
>> +     argc = parse_options(argc, argv, NULL,
>> +                     options, builtin_bundle_usage,
>> +                     PARSE_OPT_KEEP_ARGV0 | PARSE_OPT_KEEP_UNKNOWN);
>
> Doesn't this make the usage completely confusing?  Before, if I wanted
> to create bundle named "verify", I could write
>
>        git bundle create verify
>
> Afterwards, not only does that not work, but if I make a typo and
> write
>
>        git bundle ceate verify
>
> then it will act like "git bundle verify ceate".

No it won't -- it'll just print out the usage because "verify" and "create" are mutually exclusive options. Sure, you can't create a bundle named "verify", but that's the compromise you'll have to make if you don't want to type out "--" with each option, no?

> I am starting to suspect the first half of patch 1/2 was not such a
> great idea, either. :)  Do you have other examples of how it would be
> used?

Hehe, my lack of foresight is disturbing as usual. I'm not yet sure it'll be useful.

-- Ram