Re: [PATCH] ls-tree: fix --no-full-name

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

René Scharfe <l.s.r@web.de> writes:

> Some test expectations in t0040 and t1502 would have to be adjusted.
>
> This reveals, by the way, that t1502 doesn't yet exercise the "!" flag
> of "git rev-parse --parseopt" that turns on PARSE_OPT_NONEG.  I find
> the "-h, --[no-]help" option strangely amusing..
>
> --- >8 ----
> Subject: [PATCH] parse-options: show negatability of options in short help
>
> Add a "[no-]" prefix to options without the flag PARSE_OPT_NONEG to
> document the fact that you can negate them.
>
> This looks a bit strange for options that already start with "no-", e.g.
> for the option --no-name of git show-branch:
>
>     --[no-]no-name        suppress naming strings
>
> You can actually use --no-no-name as an alias of --name, so the short
> help is not wrong.  If we strip off any of the "no-"s, we lose either
> the ability to see if the remaining one belongs to the documented
> variant or to see if it can be negated.
>
> Signed-off-by: René Scharfe <l.s.r@web.de>
> ---
>  parse-options.c               | 10 ++++-
>  t/t0040-parse-options.sh      | 44 ++++++++++---------
>  t/t1502-rev-parse-parseopt.sh | 80 ++++++++++++++++++++---------------
>  3 files changed, 77 insertions(+), 57 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index f8a155ee13..6323ca191d 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -1136,8 +1136,14 @@ static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t
>  		}
>  		if (opts->long_name && opts->short_name)
>  			pos += fprintf(outfile, ", ");
> -		if (opts->long_name)
> -			pos += fprintf(outfile, "--%s", opts->long_name);
> +		if (opts->long_name) {
> +			const char *long_name = opts->long_name;
> +			if (opts->flags & PARSE_OPT_NONEG)
> +				pos += fprintf(outfile, "--%s", long_name);
> +			else
> +				pos += fprintf(outfile, "--[no-]%s", long_name);
> +		}

This is a good starting point, but we should at least exempt
OPT_BOOL from this exercise, I would think, because ...

>      A helper function for the parse-options API.
>
> -    --yes                 get a boolean
> +    --[no-]yes            get a boolean

... they are designed to be prefixed with an optional "no-".


> -    -D, --no-doubt        begins with 'no-'
> +    -D, --[no-]no-doubt   begins with 'no-'

Hmph, I really really loved the neat trick to allow "no-doubt"
option to be "positivised" by _dropping_ the leading "no-" at around
0f1930c5 (parse-options: allow positivation of options starting,
with no-, 2012-02-25).

>  EOF

Many of the above are amusing and served as good demonstration to
show the blast radius, but it seems that most of them should be
marked with PARSE_OPT_NONEG.

> diff --git a/t/t1502-rev-parse-parseopt.sh b/t/t1502-rev-parse-parseopt.sh
> index dd811b7fb4..0a67e2dd4f 100755
> --- a/t/t1502-rev-parse-parseopt.sh
> +++ b/t/t1502-rev-parse-parseopt.sh
> @@ -64,33 +64,38 @@ test_expect_success 'test --parseopt help output' '
>  |
>  |    some-command does foo and bar!
>  |
> -|    -h, --help            show the help
> -|    --foo                 some nifty option --foo
> -|    --bar ...             some cool option --bar with an argument
> -|    -b, --baz             a short and long option
> +|    -h, --[no-]help       show the help

Indeed it is amusing, but we probably should give PARSE_OPT_NONEG
appropriately, instead of changing the expectations, for many of the
changes we see here, I think.