Re: [PATCH v2 3/5] Help.c add --guide option
- From
Philip Oakley <philipoakley@iee.org>
- Date
- Mar 3, 2013, 23:44 UTC
- Message-ID
- <667B2BF898CC4491BE02C1142472A833@PhilipOakley>
- In-Reply-To
- <7vk3poyryc.fsf@alter.siamese.dyndns.org>
From: "Junio C Hamano" <gitster@pobox.com> Sent: Sunday, March 03, 2013 11:38 PM
Show 8 quoted lines
> 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.
True. I can add to the commit message.
Show 55 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.I was trying to minimise the apparent changes, but like you say...
Show 15 quoted lines
>
> 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;
> }Neater.
Show 8 quoted lines
>
> 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();Seems reasonable. Should that be a separate preparatory patch (likely), or joined in with the rest?
Philip