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>
This commit is contained in:
parent
8d714b11df
commit
b04ba2bb42
@ -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.
|
||||
|
@ -83,7 +83,7 @@ static int get_value(struct parse_opt_ctx_t *p,
|
||||
*(int *)opt->value &= ~opt->defval;
|
||||
return 0;
|
||||
|
||||
case OPTION_BOOLEAN:
|
||||
case OPTION_COUNTUP:
|
||||
*(int *)opt->value = unset ? 0 : *(int *)opt->value + 1;
|
||||
return 0;
|
||||
|
||||
@ -319,7 +319,7 @@ static void parse_options_check(const struct option *opts)
|
||||
err |= optbug(opts, "uses feature "
|
||||
"not supported for dashless options");
|
||||
switch (opts->type) {
|
||||
case OPTION_BOOLEAN:
|
||||
case OPTION_COUNTUP:
|
||||
case OPTION_BIT:
|
||||
case OPTION_NEGBIT:
|
||||
case OPTION_SET_INT:
|
||||
|
@ -10,7 +10,7 @@ enum parse_opt_type {
|
||||
/* options with no arguments */
|
||||
OPTION_BIT,
|
||||
OPTION_NEGBIT,
|
||||
OPTION_BOOLEAN, /* _INCR would have been a better name */
|
||||
OPTION_COUNTUP,
|
||||
OPTION_SET_INT,
|
||||
OPTION_SET_PTR,
|
||||
/* options with arguments (usually) */
|
||||
@ -21,6 +21,9 @@ enum parse_opt_type {
|
||||
OPTION_FILENAME
|
||||
};
|
||||
|
||||
/* Deprecated synonym */
|
||||
#define OPTION_BOOLEAN OPTION_COUNTUP
|
||||
|
||||
enum parse_opt_flags {
|
||||
PARSE_OPT_KEEP_DASHDASH = 1,
|
||||
PARSE_OPT_STOP_AT_NON_OPTION = 2,
|
||||
@ -122,10 +125,11 @@ struct option {
|
||||
PARSE_OPT_NOARG, NULL, (b) }
|
||||
#define OPT_NEGBIT(s, l, v, h, b) { OPTION_NEGBIT, (s), (l), (v), NULL, \
|
||||
(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 }
|
||||
#define OPT_SET_INT(s, l, v, h, i) { OPTION_SET_INT, (s), (l), (v), NULL, \
|
||||
(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, \
|
||||
(h), PARSE_OPT_NOARG, NULL, (p) }
|
||||
#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, \
|
||||
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
|
||||
* non-option arguments in argv[].
|
||||
|
Loading…
Reference in New Issue
Block a user