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

On Thu, Mar 17, 2016 at 01:21:49AM +0530, Pranit Bauva wrote:

I noticed that parse-options does not recognize the variable which is
set to -1 so as to denote the "unspecified" value.

Right. Like all of the stock parse-options handlers, it does not ever
read or understand the value passed to it by the caller. It only
increments or decrements.

I did the following changes in builtin/commit.c (in master branch not
the patch I am working on) :
 - static int verbose = -1
 - introduced a printf statement after parsing the options to print
the value of verbose.

When I ran `git commit` :
 I get the output that verbose is set to -1.

When I ran `git commit -v` :
I get the output that verbose is set to 0.

When I ran `git commit -v -v` :
I get the output that verbose is set to 1.

When I ran `git commit --no-verbose` :
I get the out that verbose is set to 0.
[...]
It seems that parse-options just increments the value without
considering the -1 flag to denote "unspecified value".

Is this a bug?

Not in parse-options, though I think setting verbose to "-1" in the
first place is wrong.

In general, parse-options does not know or care about the default values
that callers assign to variables; it just writes to them based on the
option-type specified by the caller. So the behavior for "commit",
"commit -v", and "commit -v -v" you show are perfectly reasonable.

But the one for "--no-verbose" is wrong. Parse-options has to write some
"reset" value, and it does not know what the initial default was. So it
writes 0. This is the same for options like OPT_SET_INT, and similar for
string options (where we set it to NULL).

So I think the caller choosing "-1" here as the "not set" value is the
bug.

-Peff