Re: parse-options does not recognize "unspecified" behavior

On Wed, Mar 16, 2016 at 5:23 PM, Jeff King [off-list ref] wrote:

On Thu, Mar 17, 2016 at 02:36:51AM +0530, Pranit Bauva wrote:

quoted

I agree to you on the point that parse-options should not care about
the value passed to it. But I think plainly incrementing the value of
the variable is not a very nice way. I have an another approach to it.
The parse-options will first store a temporary structure. If there is
some changes (not the "--no-" ones) then it sets the respective
variable in temporary structure to the set value. If "--no-" is passed
then it writes the "reset" value to the respective variable in
temporary structure. If nothing about that options is specified then
it copies the respective variable from original to temporary. After
completing the entire process, it can copy temporary structure to the
original structure.

What are your opinions about this?

I don't think that would produce the wrong behavior, but it seems like a
very complicated solution to a problem that can easily be solved by just
following the usual conventions (that verbose starts at 0, options make
it go up or down, and "--no-" resets it to zero).

I agree that this is overly complicated.

Perhaps it would make more sense if I understood what your goal was in
setting verbose to -1 in the first place.

The goal comes from his GSoC microproject. Specifically, Pranit wants
an "unspecified" value. The reason is that he is adding a
commit.verbose=<level> config variable to back the existing git-commit
--verbose option. Any use of --verbose (one or more times) or
--no-verbose should override the config.verbose value altogether, so
he wants a way to know if --verbose or --no-verbose was used; hence
the "unspecified" value. And, really, this issue isn't necessarily
specific to git-commit. It could apply to any command that understands
verbosity levels and wants to be able to get them from both a config
variable and a command-line option.

A much easier solution would be to update OPT_VERBOSE() to understand
that negative values are "unspecified", and then --verbose would
(pseudocode):

    if (value < 0)
        value = 0
    value++;

and --no-verbose would:

    value = 0

That should be compatible with existing clients of OPT__VERBOSE()
which initialize the value to 0, and should satisfy Pranit's case; he
can initialize it to -1, and if it is still -1 when option parsing is
done, then he knows that neither --verbose nor --no-verbose was seen.