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 <gitster@pobox.com>
			
			
				maint
			
			
		
							parent
							
								
									8d714b11df
								
							
						
					
					
						commit
						b04ba2bb42
					
				|  | @ -135,9 +135,14 @@ There are some macros to easily define options: | ||||||
| 	describes the group or an empty string. | 	describes the group or an empty string. | ||||||
| 	Start the description with an upper-case letter. | 	Start the description with an upper-case letter. | ||||||
|  |  | ||||||
| `OPT_BOOLEAN(short, long, &int_var, description)`:: | `OPT_BOOL(short, long, &int_var, description)`:: | ||||||
| 	Introduce a boolean option. | 	Introduce a boolean option. `int_var` is set to one with | ||||||
| 	`int_var` is incremented on each use. | 	`--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)`:: | `OPT_BIT(short, long, &int_var, description, mask)`:: | ||||||
| 	Introduce a boolean option. | 	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`. | 	If used, `int_var` is bitwise-anded with the inverted `mask`. | ||||||
|  |  | ||||||
| `OPT_SET_INT(short, long, &int_var, description, integer)`:: | `OPT_SET_INT(short, long, &int_var, description, integer)`:: | ||||||
| 	Introduce a boolean option. | 	Introduce an integer option. | ||||||
| 	If used, set `int_var` to `integer`. | 	`int_var` is set to `integer` with `--option`, and | ||||||
|  | 	reset to zero with `--no-option`. | ||||||
|  |  | ||||||
| `OPT_SET_PTR(short, long, &ptr_var, description, ptr)`:: | `OPT_SET_PTR(short, long, &ptr_var, description, ptr)`:: | ||||||
| 	Introduce a boolean option. | 	Introduce a boolean option. | ||||||
|  |  | ||||||
|  | @ -83,7 +83,7 @@ static int get_value(struct parse_opt_ctx_t *p, | ||||||
| 			*(int *)opt->value &= ~opt->defval; | 			*(int *)opt->value &= ~opt->defval; | ||||||
| 		return 0; | 		return 0; | ||||||
|  |  | ||||||
| 	case OPTION_BOOLEAN: | 	case OPTION_COUNTUP: | ||||||
| 		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1; | 		*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1; | ||||||
| 		return 0; | 		return 0; | ||||||
|  |  | ||||||
|  | @ -319,7 +319,7 @@ static void parse_options_check(const struct option *opts) | ||||||
| 			err |= optbug(opts, "uses feature " | 			err |= optbug(opts, "uses feature " | ||||||
| 					"not supported for dashless options"); | 					"not supported for dashless options"); | ||||||
| 		switch (opts->type) { | 		switch (opts->type) { | ||||||
| 		case OPTION_BOOLEAN: | 		case OPTION_COUNTUP: | ||||||
| 		case OPTION_BIT: | 		case OPTION_BIT: | ||||||
| 		case OPTION_NEGBIT: | 		case OPTION_NEGBIT: | ||||||
| 		case OPTION_SET_INT: | 		case OPTION_SET_INT: | ||||||
|  |  | ||||||
|  | @ -10,7 +10,7 @@ enum parse_opt_type { | ||||||
| 	/* options with no arguments */ | 	/* options with no arguments */ | ||||||
| 	OPTION_BIT, | 	OPTION_BIT, | ||||||
| 	OPTION_NEGBIT, | 	OPTION_NEGBIT, | ||||||
| 	OPTION_BOOLEAN, /* _INCR would have been a better name */ | 	OPTION_COUNTUP, | ||||||
| 	OPTION_SET_INT, | 	OPTION_SET_INT, | ||||||
| 	OPTION_SET_PTR, | 	OPTION_SET_PTR, | ||||||
| 	/* options with arguments (usually) */ | 	/* options with arguments (usually) */ | ||||||
|  | @ -21,6 +21,9 @@ enum parse_opt_type { | ||||||
| 	OPTION_FILENAME | 	OPTION_FILENAME | ||||||
| }; | }; | ||||||
|  |  | ||||||
|  | /* Deprecated synonym */ | ||||||
|  | #define OPTION_BOOLEAN OPTION_COUNTUP | ||||||
|  |  | ||||||
| enum parse_opt_flags { | enum parse_opt_flags { | ||||||
| 	PARSE_OPT_KEEP_DASHDASH = 1, | 	PARSE_OPT_KEEP_DASHDASH = 1, | ||||||
| 	PARSE_OPT_STOP_AT_NON_OPTION = 2, | 	PARSE_OPT_STOP_AT_NON_OPTION = 2, | ||||||
|  | @ -122,10 +125,11 @@ struct option { | ||||||
| 				      PARSE_OPT_NOARG, NULL, (b) } | 				      PARSE_OPT_NOARG, NULL, (b) } | ||||||
| #define OPT_NEGBIT(s, l, v, h, b)   { OPTION_NEGBIT, (s), (l), (v), NULL, \ | #define OPT_NEGBIT(s, l, v, h, b)   { OPTION_NEGBIT, (s), (l), (v), NULL, \ | ||||||
| 				      (h), PARSE_OPT_NOARG, NULL, (b) } | 				      (h), PARSE_OPT_NOARG, NULL, (b) } | ||||||
| #define OPT_BOOLEAN(s, l, v, h)     { OPTION_BOOLEAN, (s), (l), (v), NULL, \ | #define OPT_COUNTUP(s, l, v, h)     { OPTION_COUNTUP, (s), (l), (v), NULL, \ | ||||||
| 				      (h), PARSE_OPT_NOARG } | 				      (h), PARSE_OPT_NOARG } | ||||||
| #define OPT_SET_INT(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, \ | #define OPT_SET_INT(s, l, v, h, i)  { OPTION_SET_INT, (s), (l), (v), NULL, \ | ||||||
| 				      (h), PARSE_OPT_NOARG, NULL, (i) } | 				      (h), PARSE_OPT_NOARG, NULL, (i) } | ||||||
|  | #define OPT_BOOL(s, l, v, h)        OPT_SET_INT(s, l, v, h, 1) | ||||||
| #define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, \ | #define OPT_SET_PTR(s, l, v, h, p)  { OPTION_SET_PTR, (s), (l), (v), NULL, \ | ||||||
| 				      (h), PARSE_OPT_NOARG, NULL, (p) } | 				      (h), PARSE_OPT_NOARG, NULL, (p) } | ||||||
| #define OPT_INTEGER(s, l, v, h)     { OPTION_INTEGER, (s), (l), (v), "n", (h) } | #define OPT_INTEGER(s, l, v, h)     { OPTION_INTEGER, (s), (l), (v), "n", (h) } | ||||||
|  | @ -149,6 +153,8 @@ struct option { | ||||||
| 	{ OPTION_CALLBACK, (s), (l), (v), "when", (h), PARSE_OPT_OPTARG, \ | 	{ OPTION_CALLBACK, (s), (l), (v), "when", (h), PARSE_OPT_OPTARG, \ | ||||||
| 		parse_opt_color_flag_cb, (intptr_t)"always" } | 		parse_opt_color_flag_cb, (intptr_t)"always" } | ||||||
|  |  | ||||||
|  | /* Deprecated synonym */ | ||||||
|  | #define OPT_BOOLEAN OPT_COUNTUP | ||||||
|  |  | ||||||
| /* parse_options() will filter out the processed options and leave the | /* parse_options() will filter out the processed options and leave the | ||||||
|  * non-option arguments in argv[]. |  * non-option arguments in argv[]. | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Junio C Hamano
						Junio C Hamano