Junio C Hamano <gitster@xxxxxxxxx> writes:
> It triggers this codepath in get_value():
>
> case OPTION_BOOLEAN:
> *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
> return 0;
>
> and ends up incrementing it to zero.
>
> I wonder what would break if we simply change this to:
>
> case OPTION_BOOLEAN:
> *(int *)opt->value = !unset;
> return 0;
>
> Damn it, it is called BOOLEAN, and naïvely I would think --option would
> set it to 1, --no-option would set it to 0, and not having --option nor
> --no-option would leave the associated variable alone, but apparently that
> is not what is happening.
>
> Pierre, do you remember why this code is implemented this way? The
> "increment if we have --option" semantics seems to date back to 4a59fd1
> (Add a simple option parser., 2007-10-15) which is the day one of the
> history of parse-options.
I think that this came from a misguided attempt to do:
-v
-v -v
-v -v -v
to cumulatively record the desired verbosity levels.
Originally we did not have OPT__VERBOSITY() nor OPT_VERBOSE() so it is
understandable that OPT_VERBOSE() was implemented in terms of this
OPT_BOOLEAN() construct.
I think all parse_options() users that do support the verbosity level is
either using OPT__VERBOSE() or OPT_VERBOSITY() these days, and we could
probably fix this by doing something like:
(1) Introduce OPTION_CUMULATIVE_LEVEL whose behaviour is "--no-option to
reset to zero, --option to raise by one", and fix OPTION_BOOLEAN to
"--no-option to zero, --option to one":
case OPTION_BOOLEAN:
*(int *)opt->value = !unset;
return 0;
case OPTION_CUMULATIVE_LEVEL:
*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
return 0;
(2) Use OPT_CUMULATIVE_LEVEL() instead of OPT_BOOLEAN() to implement
OPT__VERBOSE() and OPT__QUIET().
But I am inclined to postpone such a change til after 1.6.2.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
