From: Ramkumar Ramachandra <artagnon@gmail.com>
Date: Thu, 08 Dec 2011 16:47:39 GMT
Subject: Re: [PATCH 2/2] bundle: rewrite builtin to use parse-options
Message-ID: <CALkWK0mmjKSzSbxq2i7=JvcB4LTro-MYDCwQLUUwqcf8qS0zPA@mail.gmail.com>
In-Reply-To: <20111208163946.GB2394@elie.hsd1.il.comcast.net>

Hi,

Jonathan Nieder wrote:
> 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