Re: [PATCH 4/4] parse-options: clarify PARSE_OPT_NONEG does not reject negative numbers

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

"Michael Montalbo via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Michael Montalbo <mmontalbo@gmail.com>
>
> The name "NONEG" can be misread as "no negative [values]" when it
> actually means "no [boolean] negation" (the --no-* form).
>
> When --inter-hunk-context and -U/--unified were converted from a
> custom parser to OPT_INTEGER_F with PARSE_OPT_NONEG in d473e2e0e8
> and 16ed6c97cc, the implicit rejection of negative values (via
> isdigit() in the old opt_arg() parser) was silently lost. The
> previous commits in this series fix the resulting bugs.

I do not think _NONEG has anything to do with the bug.  It was
purely to reject --no-unified and --no-inter-hunk-context.

And there was no change to remove PARSE_OPT_NONEG from anywhere and
use OPT_UNSIGNED instead to fix any of the bugs fixed in this
series, ...

>
> Add a clarifying note to the flag documentation.
>
> Signed-off-by: Michael Montalbo <mmontalbo@gmail.com>
> ---
>  parse-options.h | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/parse-options.h b/parse-options.h
> index 706de9729f..c0a3a3dcae 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -116,7 +116,10 @@ typedef int parse_opt_subcommand_fn(int argc, const char **argv,
>   *   mask of parse_opt_option_flags.
>   *   PARSE_OPT_OPTARG: says that the argument is optional (not for BOOLEANs)
>   *   PARSE_OPT_NOARG: says that this option does not take an argument
> - *   PARSE_OPT_NONEG: says that this option cannot be negated
> + *   PARSE_OPT_NONEG: says that this option cannot be negated (i.e.
> + *                   prevents --no-<option> boolean form). Does not reject
> + *                   negative numeric values like --option=-1. Use
> + *                   OPT_UNSIGNED for options that must be non-negative.

... I do not think the two additional sentences are warranted.  Stop
at clarifying what negated _means_ (i.e., rejects "--no-<option>"),
without adding what negated does _not_ mean.


>   *   PARSE_OPT_HIDDEN: this option is skipped in the default usage, and
>   *                     shown only in the full usage.
>   *   PARSE_OPT_LASTARG_DEFAULT: says that this option will take the default