Re: [PATCH v2 3/5] Help.c add --guide option

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 3, 2013, 23:38 UTC
Message-ID: <7vk3poyryc.fsf@alter.siamese.dyndns.org>
In-Reply-To: <1362342072-1412-4-git-send-email-philipoakley@iee.org>

Philip Oakley <philipoakley@iee.org> writes:

> Logic, but no actions, included.

I am not sure what you mean.  Is that to break "bisect"?

Ahh, you meant "command line is parsed but we do not actually show guides yet, which is done by later patches in this series". OK.

Show 47 quoted lines

> The --all commands option, if given, will display first.
> The --guide option's list will then be displayed.
>
> The common commands list is only displayed if neither option,
> nor a command or guide name, is given.
>
> Signed-off-by: Philip Oakley <philipoakley@iee.org>
> ---
>  builtin/help.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index d10cbed..6089d72 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -36,10 +36,12 @@ enum help_format {
>  static const char *html_path;
>  
>  static int show_all = 0;
> +static int show_guides = 0;
>  static unsigned int colopts;
>  static enum help_format help_format = HELP_FORMAT_NONE;
>  static struct option builtin_help_options[] = {
>  	OPT_COUNTUP('a', "all", &show_all, N_("print all available commands")),
> +	OPT_COUNTUP('g', "guides", &show_guides, N_("print list of useful guides")),
>  	OPT_SET_INT('m', "man", &help_format, N_("show man page"), HELP_FORMAT_MAN),
>  	OPT_SET_INT('w', "web", &help_format, N_("show manual in web browser"),
>  			HELP_FORMAT_WEB),
> @@ -49,7 +51,7 @@ static struct option builtin_help_options[] = {
>  };
>  
>  static const char * const builtin_help_usage[] = {
> -	N_("git help [--all] [--man|--web|--info] [command]"),
> +	N_("git help [--all] [--guides] [--man|--web|--info] [command]"),
>  	NULL
>  };
>  
> @@ -429,9 +431,11 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  		printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
>  		list_commands(colopts, &main_cmds, &other_cmds);
>  		printf("%s\n", _(git_more_info_string));
> +		if (!show_guides) return 0;
> +	}
> +	if (show_guides) {
>  		return 0;
>  	}
> -

Ugly.

	if (show_all) {
        	... do not touch anything here ...
		... but remove "return 0;" ...
	}

	if (show_guides) {
        	... show guides but do not "return 0" ...
	}

	if (show_all || show_guides) {
		... we were asked to do either/or --all/--guides ...
                ... and have done what we were asked to do ...
		return 0;
	}

This is a tangent, but before all of the above, cmd_help() should verify that it got no arguments (when show_all/show_guides is in effect) or it got one argument (otherwise), I think.

>  	if (!argv[0]) {
>  		printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
>  		list_common_cmds_help();