From b04ba2bb42957f63567f530e092385f085b25e63 Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 27 Sep 2011 16:56:49 -0700 Subject: parse-options: deprecate OPT_BOOLEAN It is natural to expect that an option defined with OPT_BOOLEAN() could be used in this way: int option = -1; /* unspecified */ struct option options[] = { OPT_BOOLEAN(0, "option", &option, "set option"), OPT_END() }; parse_options(ac, av, prefix, options, usage, 0); if (option < 0) ... do the default thing ... else if (!option) ... --no-option was given ... else ... --option was given ... to easily tell three cases apart: - There is no mention of the `--option` on the command line; - The variable is positively set with `--option`; or - The variable is explicitly negated with `--no-option`. Unfortunately, this is not the case. OPT_BOOLEAN() increments the variable every time `--option` is given, and resets it to zero when `--no-option` is given. As a first step to remedy this, introduce a true boolean OPT_BOOL(), and rename OPT_BOOLEAN() to OPT_COUNTUP(). To help transitioning, OPT_BOOLEAN and OPTION_BOOLEAN are defined as deprecated synonyms to OPT_COUNTUP and OPTION_COUNTUP respectively. This is what db7244b (parse-options new features., 2007-11-07) from four years ago started by marking OPTION_BOOLEAN as "INCR would have been a better name". Some existing users do depend on the count-up semantics; for example, users of OPT__VERBOSE() could use it to raise the verbosity level with repeated use of `-v` on the command line, but they probably should be rewritten to use OPT__VERBOSITY() instead these days. I suspect that some users of OPT__FORCE() may also use it to implement different level of forcibleness but I didn't check. On top of this patch, here are the remaining clean-up tasks that other people can help: - Look at each hit in "git grep -e OPT_BOOLEAN"; trace all uses of the value that is set to the underlying variable, and if it can proven that the variable is only used as a boolean, replace it with OPT_BOOL(). If the caller does depend on the count-up semantics, replace it with OPT_COUNTUP() instead. - Same for OPTION_BOOLEAN; replace it with OPTION_SET_INT and arrange to set 1 to the variable for a true boolean, and otherwise replace it with OPTION_COUNTUP. - Look at each hit in "git grep -e OPT__VERBOSE -e OPT__QUIET" and see if they can be replaced with OPT__VERBOSITY(). I'll follow this message up with a separate patch as an example. Signed-off-by: Junio C Hamano --- Documentation/technical/api-parse-options.txt | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'Documentation/technical') diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index f6a4a361bd..acf17607df 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -135,9 +135,14 @@ There are some macros to easily define options: describes the group or an empty string. Start the description with an upper-case letter. -`OPT_BOOLEAN(short, long, &int_var, description)`:: - Introduce a boolean option. - `int_var` is incremented on each use. +`OPT_BOOL(short, long, &int_var, description)`:: + Introduce a boolean option. `int_var` is set to one with + `--option` and set to zero with `--no-option`. + +`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`. `OPT_BIT(short, long, &int_var, description, mask)`:: Introduce a boolean option. @@ -148,8 +153,9 @@ There are some macros to easily define options: If used, `int_var` is bitwise-anded with the inverted `mask`. `OPT_SET_INT(short, long, &int_var, description, integer)`:: - Introduce a boolean option. - If used, set `int_var` to `integer`. + Introduce an integer option. + `int_var` is set to `integer` with `--option`, and + reset to zero with `--no-option`. `OPT_SET_PTR(short, long, &ptr_var, description, ptr)`:: Introduce a boolean option. -- cgit v1.3 From 6acec0380bfcd5e3f91c9100bf4d415db8f7acfd Mon Sep 17 00:00:00 2001 From: René Scharfe Date: Wed, 28 Sep 2011 19:44:30 +0200 Subject: parseopt: add OPT_NOOP_NOARG Add OPT_NOOP_NOARG, a helper macro to define deprecated options in a standard way. The help text is taken from the no-op option -r of git revert. The callback could be made to emit a (conditional?) warning later. And we could also add OPT_NOOP (requiring an argument) etc. as needed. Signed-off-by: Rene Scharfe Signed-off-by: Junio C Hamano --- Documentation/technical/api-parse-options.txt | 5 +++++ parse-options-cb.c | 5 +++++ parse-options.h | 6 ++++++ t/t0040-parse-options.sh | 2 +- test-parse-options.c | 1 + 5 files changed, 18 insertions(+), 1 deletion(-) (limited to 'Documentation/technical') diff --git a/Documentation/technical/api-parse-options.txt b/Documentation/technical/api-parse-options.txt index acf17607df..4b92514f60 100644 --- a/Documentation/technical/api-parse-options.txt +++ b/Documentation/technical/api-parse-options.txt @@ -204,6 +204,11 @@ There are some macros to easily define options: "auto", set `int_var` to 1 if stdout is a tty or a pager, 0 otherwise. +`OPT_NOOP_NOARG(short, long)`:: + Introduce an option that has no effect and takes no arguments. + Use it to hide deprecated options that are still to be recognized + and ignored silently. + The last element of the array must be `OPT_END()`. diff --git a/parse-options-cb.c b/parse-options-cb.c index 6db0921fc1..0de5fb168a 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -123,3 +123,8 @@ int parse_opt_string_list(const struct option *opt, const char *arg, int unset) string_list_append(v, xstrdup(arg)); return 0; } + +int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) +{ + return 0; +} diff --git a/parse-options.h b/parse-options.h index 22c0273052..2e811dc7da 100644 --- a/parse-options.h +++ b/parse-options.h @@ -153,6 +153,11 @@ struct option { { OPTION_CALLBACK, (s), (l), (v), "when", (h), PARSE_OPT_OPTARG, \ parse_opt_color_flag_cb, (intptr_t)"always" } +#define OPT_NOOP_NOARG(s, l) \ + { OPTION_CALLBACK, (s), (l), NULL, NULL, \ + "no-op (backward compatibility)", \ + PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb } + /* Deprecated synonym */ #define OPT_BOOLEAN OPT_COUNTUP @@ -216,6 +221,7 @@ extern int parse_opt_verbosity_cb(const struct option *, const char *, int); extern int parse_opt_with_commit(const struct option *, const char *, int); extern int parse_opt_tertiary(const struct option *, const char *, int); extern int parse_opt_string_list(const struct option *, const char *, int); +extern int parse_opt_noop_cb(const struct option *, const char *, int); #define OPT__VERBOSE(var, h) OPT_BOOLEAN('v', "verbose", (var), (h)) #define OPT__QUIET(var, h) OPT_BOOLEAN('q', "quiet", (var), (h)) diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index 007f39d5e1..a1e4616feb 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -87,7 +87,7 @@ EOF test_expect_success 'long options' ' test-parse-options --boolean --integer 1729 --boolean --string2=321 \ --verbose --verbose --no-dry-run --abbrev=10 --file fi.le\ - > output 2> output.err && + --obsolete > output 2> output.err && test ! -s output.err && test_cmp expect output ' diff --git a/test-parse-options.c b/test-parse-options.c index 91a5701657..36487c402b 100644 --- a/test-parse-options.c +++ b/test-parse-options.c @@ -54,6 +54,7 @@ int main(int argc, const char **argv) OPT_STRING(0, "string2", &string, "str", "get another string"), OPT_STRING(0, "st", &string, "st", "get another string (pervert ordering)"), OPT_STRING('o', NULL, &string, "str", "get another string"), + OPT_NOOP_NOARG(0, "obsolete"), OPT_SET_PTR(0, "default-string", &string, "set string to default", (unsigned long)"default"), OPT_STRING_LIST(0, "list", &list, "str", "add str to list"), -- cgit v1.3