Re: [PATCH 1/2] clone: do not ignore --no-hardlinks

Junio C Hamano [off-list ref] 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.