From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 17 Nov 2009 01:02:27 GMT
Subject: Re: [PATCH RFC] git remote: Separate usage strings for subcommands
Message-ID: <7vocn2klp8.fsf@alter.siamese.dyndns.org>
In-Reply-To: <32c343770911151343y1ed0aak83b43a4ab57eb6ab@mail.gmail.com>

Tim Henigan <tim.henigan@gmail.com> writes:

> All usage strings are still only located at the top of file.  However,
> separate usage string arrays have been created for each subcommand.
>
> Does this look like a sane way to structure the code?
>
> If anyone else should be added to the CC list, please let me know.
>
>
>  builtin-remote.c |   57 +++++++++++++++++++++++++++++++++--------------------
>  1 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/builtin-remote.c b/builtin-remote.c
> index 0777dd7..ec65a4b 100644
> --- a/builtin-remote.c
> +++ b/builtin-remote.c
> @@ -7,18 +7,35 @@
>  #include "run-command.h"
>  #include "refs.h"
>
> +#define REMOTE_BARE_USAGE	"git remote [-v | --verbose]"
> +#define REMOTE_ADD_USAGE	"git remote add [-t <branch>] [-m <master>]
> [-f] [--mirror] <name> <url>"
> +#define REMOTE_RENAME_USAGE	"git remote rename <old> <new>"
> +#define REMOTE_RM_USAGE		"git remote rm <name>"
> +#define REMOTE_SETHEAD_USAGE	"git remote set-head <name> [-a | -d | <branch>]"
> +#define REMOTE_SHOW_USAGE	"git remote show [-n] <name>"
> +#define REMOTE_PRUNE_USAGE	"git remote prune [-n | --dry-run] <name>"
> +#define REMOTE_UPDATE_USAGE	"git remote [-v | --verbose] update [-p |
> --prune] [group]"
> +
>  static const char * const builtin_remote_usage[] = {
> -	"git remote [-v | --verbose]",
> -	"git remote add [-t <branch>] [-m <master>] [-f] [--mirror] <name> <url>",
> -	"git remote rename <old> <new>",
> -	"git remote rm <name>",
> -	"git remote set-head <name> [-a | -d | <branch>]",
> -	"git remote show [-n] <name>",
> -	"git remote prune [-n | --dry-run] <name>",
> -	"git remote [-v | --verbose] update [-p | --prune] [group]",
> +	REMOTE_BARE_USAGE,
> +	REMOTE_ADD_USAGE,
> +	REMOTE_RENAME_USAGE,
> +	REMOTE_RM_USAGE,
> +	REMOTE_SETHEAD_USAGE,
> +	REMOTE_SHOW_USAGE,
> +	REMOTE_PRUNE_USAGE,
> +	REMOTE_UPDATE_USAGE,
>  	NULL
>  };
> ...
> +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 ...

> @@ -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