On Mon, Nov 16, 2009 at 8:02 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Tim Henigan <tim.henigan@xxxxxxxxx> writes:
>
>> ...
>> +static const char * const builtin_remote_add_usage[] = {
>> REMOTE_ADD_USAGE, NULL };
>> ...
>
> I am not sure about the value of reusing option string like this, and for
> all other subcommands the same comment applies. For example, in the case
> of "remote add -h", you would use
>
> "git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>"
> from REMOTE_ADD_USAGE, but ...
Do you object to the new #defines for the strings? I added them since we now
really need to construct the usage string for 2 separate uses:
(1) When 'git remote -h' is invoked, we need to return the usage
string showing
all subcommands.
(2) When 'git remote <subcommand> -h' is invoked, we need to return the
usage for just that command.
It doesn't make sense to me to maintain separate strings for the two use cases.
>> @@ -70,7 +87,6 @@ static int add(int argc, const char **argv)
>> int i;
>>
>> struct option options[] = {
>> - OPT_GROUP("add specific options"),
>> OPT_BOOLEAN('f', "fetch", &fetch, "fetch the remote branches"),
>> OPT_CALLBACK('t', "track", &track, "branch",
>> "branch(es) to track", opt_parse_track),
>> @@ -79,11 +95,11 @@ static int add(int argc, const char **argv)
>> OPT_END()
>> };
>>
>> - argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
>> + argc = parse_options(argc, argv, NULL, options, builtin_remote_add_usage,
>> 0);
>
> ... the options list is used to reproduce the information in a major part
> of that string already. So I would prefer builtin_remote_add_usage[] to
> be something like:
>
> "git remote add [<options>...] <name> <url>"
>
> It is in line with a discussion we had earlier:
>
> http://thread.gmane.org/gmane.comp.version-control.git/129906/focus=130646
I planned to make this change in a separate (or maybe just a later
version) patch,
but first I wanted to be sure my other changes were sound. I will send a new
version that includes this change.
Should this change be made to the man page as well?
>> @@ -1334,7 +1348,6 @@ static int show_all(void)
>> int cmd_remote(int argc, const char **argv, const char *prefix)
>> {
>> struct option options[] = {
>> - OPT__VERBOSE(&verbose),
>> OPT_END()
>> };
>> int result;
I did find one problem with the above portion of the patch. With this change
'-v' was no longer recognized as an option. I will remove this change in the
next version. Keeping it means that '-v' will still be reported in the
'git remote -h' usage string, but I don't see an easy way to remove the string
without removing the feature.
--
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