Re: [PATCH v3 2/9] help: correct usage & behavior of "git help --guides"

[Junio C Hamano <gitster@pobox.com>]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> The "We're done. Ignore any remaining args" comment added in
> 15f7d494380 (builtin/help.c: split "-a" processing into two,
> 2013-04-02) can now be removed, it's obvious that we're asserting the
> behavior with the check of "argc".

That is true for show_guides, but not for show_all, no?

> @@ -552,6 +553,11 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  			builtin_help_usage, 0);
>  	parsed_help_format = help_format;
>  
> +	/* Options that take no further arguments */
> +	if (argc && show_guides)
> +		usage_msg_opt(_("--guides cannot be combined with other options"),
> +			      builtin_help_usage, builtin_help_options);

This is a great addition. "git help -a foo" does not seem to fail,
but shouldn't it be given the same treatment before we can remove
the "We're done" comment?

>  	if (show_all) {
>  		git_config(git_help_config, NULL);
>  		if (verbose) {
> @@ -582,9 +588,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
>  
>  	if (show_all || show_guides) {
>  		printf("%s\n", _(git_more_info_string));
> -		/*
> -		* We're done. Ignore any remaining args
> -		*/
>  		return 0;
>  	}
>  
> diff --git a/t/t0012-help.sh b/t/t0012-help.sh
> index 5679e29c624..0525ec3ee58 100755
> --- a/t/t0012-help.sh
> +++ b/t/t0012-help.sh
> @@ -34,6 +34,10 @@ test_expect_success 'basic help commands' '
>  	git help -a >/dev/null
>  '
>  
> +test_expect_success 'invalid usage' '
> +	test_expect_code 129 git help -g add
> +'
> +
>  test_expect_success "works for commands and guides by default" '
>  	configure_help &&
>  	git help status &&