The reason to make it respect "unspecified" values is to give the ability to differentiate whether `--option` or `--no-option` was specified at all. "unspecified" values should be in the form of negative values. If initial value is set to negative and `--option` specified then it will reflect the number of occurrences (counting done from 0), if `--no-option` is specified then it will reflect 0 and if nothing at all is given then it will retain its negative value. This change will not affect existing users of COUNTUP at all, as long as they use the initial value of 0 (or more). Force uses the "unspecified" value in conjunction with OPT__FORCE in builtin/clean.c in a different way to handle its config which munges the "-1" into 0 before we get to parse_options. To test this behavior "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@xxxxxxxx> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx> Helped-by: Junio C Hamano <gitster@xxxxxxxxx> Signed-off-by: Pranit Bauva <pranit.bauva@xxxxxxxxx> --- The discussion about this patch: [1] : http://thread.gmane.org/gmane.comp.version-control.git/289027 Previous version of the patch: [v10] : http://thread.gmane.org/gmane.comp.version-control.git/288820 [v9] : http://thread.gmane.org/gmane.comp.version-control.git/288820 [v1] : http://thread.gmane.org/gmane.comp.version-control.git/289061 Changes introduced: Use a different language in commit message to make the change and its utility more clear. Also added some points as to where this patch could break but it doesn't. Include tests to test this behavior by modifying test-parse-options.c and t/t0040-parse-options.sh as requested by SZEDER and reminded by Eric. Please Note: The diff might seem improper especially the part where I have introduced some continuous lines but this is a logical error by git diff (nothing could be done about it) and thus the changes will be clearly visible with the original file itself. --- Documentation/technical/api-parse-options.txt | 8 ++++-- parse-options.c | 8 +++++- t/t0040-parse-options.sh | 39 ++++++++++++++++++++------- test-parse-options.c | 3 ++- 4 files changed, 44 insertions(+), 14 deletions(-) diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index 5f0757d..8908bf7 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -144,8 +144,12 @@ There are some macros to easily define options: `OPT_COUNTUP(short, long, &int_var, description)`:: Introduce a count-up option. - `int_var` is incremented on each use of `--option`, and - reset to zero with `--no-option`. + Each use of `--option` increments `int_var`, starting from zero + (even if initially negative), and `--no-option` resets it to + zero. To determine if `--option` or `--no-option` was set at + all, set `int_var` to a negative value, and if it is still + negative after parse_options(), then neither `--option` nor + `--no-option` was seen. `OPT_BIT(short, long, &int_var, description, mask)`:: Introduce a boolean option. diff --git a/parse-options.c b/parse-options.c index 47a9192..86d30bd 100644 --- a/parse-options.c +++ b/parse-options.c @@ -110,7 +110,13 @@ static int get_value(struct parse_opt_ctx_t *p, return 0; case OPTION_COUNTUP: - *(int *)opt->value = unset ? 0 : *(int *)opt->value + 1; + if (unset) + *(int *)opt->value = 0; + else { + if (*(int *)opt->value < 0) + *(int *)opt->value = 0; + *(int *)opt->value += 1; + } return 0; case OPTION_SET_INT: diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 2dcbdc0..275bb64 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -63,7 +63,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -211,7 +211,7 @@ magnitude: 0 timestamp: 0 string: 123 abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -234,7 +234,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -263,7 +263,7 @@ magnitude: 0 timestamp: 0 string: 123 abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -302,7 +302,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -322,7 +322,7 @@ magnitude: 0 timestamp: 1 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 1 dry run: no file: (not set) @@ -344,7 +344,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -373,7 +373,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -398,7 +398,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -429,7 +429,7 @@ magnitude: 0 timestamp: 0 string: (not set) abbrev: 7 -verbose: 0 +verbose: -1 quiet: 0 dry run: no file: (not set) @@ -454,6 +454,25 @@ dry run: no file: (not set) EOF +test_expect_success 'OPT_COUNTUP() resets to 0 with --no- flag' ' + test-parse-options --no-verbose >output 2>output.err && + test_must_be_empty output.err && + test_cmp expect output +' + +cat >expect <<EOF +boolean: 0 +integer: 0 +magnitude: 0 +timestamp: 0 +string: (not set) +abbrev: 7 +verbose: -1 +quiet: 0 +dry run: no +file: (not set) +EOF + test_expect_success 'negation of OPT_NONEG flags is not ambiguous' ' test-parse-options --no-ambig >output 2>output.err && test_must_be_empty output.err && diff --git a/test-parse-options.c b/test-parse-options.c index 86afa98..f02c275 100644 --- a/test-parse-options.c +++ b/test-parse-options.c @@ -7,7 +7,8 @@ static int integer = 0; static unsigned long magnitude = 0; static unsigned long timestamp; static int abbrev = 7; -static int verbose = 0, dry_run = 0, quiet = 0; +static int verbose = -1; /* unspecified */ +static int dry_run = 0, quiet = 0; static char *string = NULL; static char *file = NULL; static int ambiguous; -- https://github.com/git/git/pull/218 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html