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

[Jeff King <peff@peff.net>]

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