Thread (8 messages) flat view 8 messages, 3 authors, 2016-06-15

Re: [PATCH/RFC] parse-options.c: make OPTION__COUNTUP consider negative values

From: Jeff King <hidden>
Date: 2016-06-15 23:08:46

On Wed, Mar 16, 2016 at 11:16:58PM +0000, Pranit Bauva wrote:
The reason to make it consider negative values or more specifically
"unspecified" values is to differentiate between the option passed
once, multiple times or with --no-option. This makes the receiver
know what actually happened with the arguments which is particularly
required with option have multiple levels of that option.

Eg. :
initialize verbose = -1
`git commit` => verbose = -1
`git commit -v` => verbose = 1
`git commit -v -v` => verbose = 1
`git commit --no-verbose` => verbose = 0
This second to last example would be 2, right?

That aside, this patch does mean that one can no longer use
OPT_COUNTUP() for negative values (i.e., the caller must start it at
either 0 or 1, and it must always go up from there).

And we would need to verify that all of the existing callers are OK with
this. Did you check that that (not rhetorical; I suspect they are all
OK, but somebody needs to check)?

We are also changing semantics without changing the interface, which
means any topics in flight (that you _cannot_ review, because you have
not seen them yet) may be subtly broken. To me that is not an absolute
deal-breaker, but something to weigh against the utility of the change.

When looking more carefully at builtin/commit.c for the other thread, it
occurred to me that OPT_BOOL might be a better fit for commit's "-v". It
really is a boolean "show the diff or not" and thus unlike the other
"make me more verbose". And OPT_BOOL already has the behavior you want,
I think.

-Peff
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help