parse-options.c: make OPTION_COUNTUP respect "unspecified" values

OPT_COUNTUP() merely increments the counter upon --option, and resets it to 0 upon --no-option, which means that there is no "unspecified" value with which a client can initialize the counter to determine whether or not --[no]-option was seen at all. Make OPT_COUNTUP() treat any negative number as an "unspecified" value to address this shortcoming. In particular, if a client initializes the counter to -1, then if it is still -1 after parse_options(), then neither --option nor --no-option was seen; if it is 0, then --no-option was seen last, and if it is 1 or greater, than --option was seen last. This change does not affect the behavior of existing clients because they all use the initial value of 0 (or more). Note that builtin/clean.c initializes the variable used with OPT__FORCE (which uses OPT_COUNTUP()) to a negative value, but it is set to either 0 or 1 by reading the configuration before the code calls parse_options(), i.e. as far as parse_options() is concerned, the initial value of the variable is not negative. To test this behavior, in test-parse-options.c, "verbose" is set to "unspecified" while quiet is set to 0 which will test the new behavior with all sets of values. Helped-by: Jeff King <peff@peff.net> Helped-by: Eric Sunshine <sunshine@sunshineco.com> Helped-by: Junio C Hamano <gitster@pobox.com> Signed-off-by: Pranit Bauva <pranit.bauva@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Pranit Bauva committed May 5, 2016 at 15:20 UTC e0070e8bd575c069ddc0a469c43076290210e79e
4 files changed +24 -17
Documentation/technical/api-parse-options.txt
+6 -2
@@ -144,8 +144,12 @@ There are some macros to easily define options:
144
145 `OPT_COUNTUP(short, long, &int_var, description)`::
146 Introduce a count-up option.
147 - `int_var` is incremented on each use of `--option`, and
148 - reset to zero with `--no-option`.
147 + Each use of `--option` increments `int_var`, starting from zero
148 + (even if initially negative), and `--no-option` resets it to
149 + zero. To determine if `--option` or `--no-option` was encountered at
150 + all, initialize `int_var` to a negative value, and if it is still
151 + negative after parse_options(), then neither `--option` nor
152 + `--no-option` was seen.
153
154 `OPT_BIT(short, long, &int_var, description, mask)`::
155 Introduce a boolean option.
parse-options.c
+2
@@ -110,6 +110,8 @@ static int get_value(struct parse_opt_ctx_t *p,
110 return 0;
111
112 case OPTION_COUNTUP:
113 + if (*(int *)opt->value < 0)
114 + *(int *)opt->value = 0;
115 *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
116 return 0;
117
t/t0040-parse-options.sh
+14 -14
@@ -63,7 +63,7 @@ magnitude: 0
63 timestamp: 0
64 string: (not set)
65 abbrev: 7
66 -verbose: 0
66 +verbose: -1
67 quiet: 0
68 dry run: no
69 file: (not set)
@@ -211,7 +211,7 @@ magnitude: 0
211 timestamp: 0
212 string: 123
213 abbrev: 7
214 -verbose: 0
214 +verbose: -1
215 quiet: 0
216 dry run: no
217 file: (not set)
@@ -234,7 +234,7 @@ magnitude: 0
234 timestamp: 0
235 string: (not set)
236 abbrev: 7
237 -verbose: 0
237 +verbose: -1
238 quiet: 0
239 dry run: no
240 file: (not set)
@@ -263,7 +263,7 @@ magnitude: 0
263 timestamp: 0
264 string: 123
265 abbrev: 7
266 -verbose: 0
266 +verbose: -1
267 quiet: 0
268 dry run: no
269 file: (not set)
@@ -302,7 +302,7 @@ magnitude: 0
302 timestamp: 0
303 string: (not set)
304 abbrev: 7
305 -verbose: 0
305 +verbose: -1
306 quiet: 0
307 dry run: no
308 file: (not set)
@@ -322,7 +322,7 @@ magnitude: 0
322 timestamp: 1
323 string: (not set)
324 abbrev: 7
325 -verbose: 0
325 +verbose: -1
326 quiet: 1
327 dry run: no
328 file: (not set)
@@ -344,7 +344,7 @@ magnitude: 0
344 timestamp: 0
345 string: (not set)
346 abbrev: 7
347 -verbose: 0
347 +verbose: -1
348 quiet: 0
349 dry run: no
350 file: (not set)
@@ -373,7 +373,7 @@ magnitude: 0
373 timestamp: 0
374 string: (not set)
375 abbrev: 7
376 -verbose: 0
376 +verbose: -1
377 quiet: 0
378 dry run: no
379 file: (not set)
@@ -398,7 +398,7 @@ magnitude: 0
398 timestamp: 0
399 string: (not set)
400 abbrev: 7
401 -verbose: 0
401 +verbose: -1
402 quiet: 0
403 dry run: no
404 file: (not set)
@@ -429,7 +429,7 @@ magnitude: 0
429 timestamp: 0
430 string: (not set)
431 abbrev: 7
432 -verbose: 0
432 +verbose: -1
433 quiet: 0
434 dry run: no
435 file: (not set)
@@ -448,7 +448,7 @@ magnitude: 0
448 timestamp: 0
449 string: (not set)
450 abbrev: 7
451 -verbose: 0
451 +verbose: -1
452 quiet: 0
453 dry run: no
454 file: (not set)
@@ -483,7 +483,7 @@ magnitude: 0
483 timestamp: 0
484 string: (not set)
485 abbrev: 7
486 -verbose: 0
486 +verbose: -1
487 quiet: 3
488 dry run: no
489 file: (not set)
@@ -521,7 +521,7 @@ magnitude: 0
521 timestamp: 0
522 string: (not set)
523 abbrev: 7
524 -verbose: 0
524 +verbose: -1
525 quiet: 0
526 dry run: no
527 file: (not set)
@@ -540,7 +540,7 @@ magnitude: 0
540 timestamp: 0
541 string: (not set)
542 abbrev: 7
543 -verbose: 0
543 +verbose: -1
544 quiet: 0
545 dry run: no
546 file: (not set)
test-parse-options.c
+2 -1
@@ -7,7 +7,8 @@ static int integer = 0;
7 static unsigned long magnitude = 0;
8 static unsigned long timestamp;
9 static int abbrev = 7;
10 -static int verbose = 0, dry_run = 0, quiet = 0;
10 +static int verbose = -1; /* unspecified */
11 +static int dry_run = 0, quiet = 0;
12 static char *string = NULL;
13 static char *file = NULL;
14 static int ambiguous;