Skip to content

Commit 366b80b

Browse files
gitstergitster
committed
Merge branch 'jc/parseopt-command-modes'
Many commands use --dashed-option as a operation mode selector (e.g. "git tag --delete") that the user can use at most one (e.g. "git tag --delete --verify" is a nonsense) and you cannot negate (e.g. "git tag --no-delete" is a nonsense). Make it easier for users of parse_options() to enforce these restrictions. * jc/parseopt-command-modes: tag: use OPT_CMDMODE parse-options: add OPT_CMDMODE()
2 parents 5fb0e08 + e6b722d commit 366b80b

File tree

3 files changed

+68
-20
lines changed

3 files changed

+68
-20
lines changed

‎builtin/tag.c

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -436,18 +436,18 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
436436
struct ref_lock *lock;
437437
struct create_tag_options opt;
438438
char *cleanup_arg = NULL;
439-
int annotate = 0, force = 0, lines = -1, list = 0,
440-
delete = 0, verify = 0;
439+
int annotate = 0, force = 0, lines = -1;
440+
int cmdmode = 0;
441441
const char *msgfile = NULL, *keyid = NULL;
442442
struct msg_arg msg = { 0, STRBUF_INIT };
443443
struct commit_list *with_commit = NULL;
444444
struct option options[] = {
445-
OPT_BOOLEAN('l', "list", &list, N_("list tag names")),
445+
OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
446446
{ OPTION_INTEGER, 'n', NULL, &lines, N_("n"),
447447
N_("print <n> lines of each tag message"),
448448
PARSE_OPT_OPTARG, NULL, 1 },
449-
OPT_BOOLEAN('d', "delete", &delete, N_("delete tags")),
450-
OPT_BOOLEAN('v', "verify", &verify, N_("verify tags")),
449+
OPT_CMDMODE('d', "delete", &cmdmode, N_("delete tags"), 'd'),
450+
OPT_CMDMODE('v', "verify", &cmdmode, N_("verify tags"), 'v'),
451451

452452
OPT_GROUP(N_("Tag creation options")),
453453
OPT_BOOLEAN('a', "annotate", &annotate,
@@ -489,22 +489,19 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
489489
}
490490
if (opt.sign)
491491
annotate = 1;
492-
if (argc == 0 && !(delete || verify))
493-
list = 1;
492+
if (argc == 0 && !cmdmode)
493+
cmdmode = 'l';
494494

495-
if ((annotate || msg.given || msgfile || force) &&
496-
(list || delete || verify))
495+
if ((annotate || msg.given || msgfile || force) && (cmdmode != 0))
497496
usage_with_options(git_tag_usage, options);
498497

499-
if (list + delete + verify > 1)
500-
usage_with_options(git_tag_usage, options);
501498
finalize_colopts(&colopts, -1);
502-
if (list && lines != -1) {
499+
if (cmdmode == 'l' && lines != -1) {
503500
if (explicitly_enable_column(colopts))
504501
die(_("--column and -n are incompatible"));
505502
colopts = 0;
506503
}
507-
if (list) {
504+
if (cmdmode == 'l') {
508505
int ret;
509506
if (column_active(colopts)) {
510507
struct column_options copts;
@@ -523,9 +520,9 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
523520
die(_("--contains option is only allowed with -l."));
524521
if (points_at.nr)
525522
die(_("--points-at option is only allowed with -l."));
526-
if (delete)
523+
if (cmdmode == 'd')
527524
return for_each_tag_name(argv, delete_tag);
528-
if (verify)
525+
if (cmdmode == 'v')
529526
return for_each_tag_name(argv, verify_tag);
530527

531528
if (msg.given || msgfile) {

‎parse-options.c

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,42 @@ static void fix_filename(const char *prefix, const char **file)
4343
*file = xstrdup(prefix_filename(prefix, strlen(prefix), *file));
4444
}
4545

46+
static int opt_command_mode_error(const struct option *opt,
47+
const struct option *all_opts,
48+
int flags)
49+
{
50+
const struct option *that;
51+
struct strbuf message = STRBUF_INIT;
52+
struct strbuf that_name = STRBUF_INIT;
53+
54+
/*
55+
* Find the other option that was used to set the variable
56+
* already, and report that this is not compatible with it.
57+
*/
58+
for (that = all_opts; that->type != OPTION_END; that++) {
59+
if (that == opt ||
60+
that->type != OPTION_CMDMODE ||
61+
that->value != opt->value ||
62+
that->defval != *(int *)opt->value)
63+
continue;
64+
65+
if (that->long_name)
66+
strbuf_addf(&that_name, "--%s", that->long_name);
67+
else
68+
strbuf_addf(&that_name, "-%c", that->short_name);
69+
strbuf_addf(&message, ": incompatible with %s", that_name.buf);
70+
strbuf_release(&that_name);
71+
opterror(opt, message.buf, flags);
72+
strbuf_release(&message);
73+
return -1;
74+
}
75+
return opterror(opt, ": incompatible with something else", flags);
76+
}
77+
4678
static int get_value(struct parse_opt_ctx_t *p,
47-
const struct option *opt, int flags)
79+
const struct option *opt,
80+
const struct option *all_opts,
81+
int flags)
4882
{
4983
const char *s, *arg;
5084
const int unset = flags & OPT_UNSET;
@@ -83,6 +117,16 @@ static int get_value(struct parse_opt_ctx_t *p,
83117
*(int *)opt->value = unset ? 0 : opt->defval;
84118
return 0;
85119

120+
case OPTION_CMDMODE:
121+
/*
122+
* Giving the same mode option twice, although is unnecessary,
123+
* is not a grave error, so let it pass.
124+
*/
125+
if (*(int *)opt->value && *(int *)opt->value != opt->defval)
126+
return opt_command_mode_error(opt, all_opts, flags);
127+
*(int *)opt->value = opt->defval;
128+
return 0;
129+
86130
case OPTION_SET_PTR:
87131
*(void **)opt->value = unset ? NULL : (void *)opt->defval;
88132
return 0;
@@ -143,12 +187,13 @@ static int get_value(struct parse_opt_ctx_t *p,
143187

144188
static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options)
145189
{
190+
const struct option *all_opts = options;
146191
const struct option *numopt = NULL;
147192

148193
for (; options->type != OPTION_END; options++) {
149194
if (options->short_name == *p->opt) {
150195
p->opt = p->opt[1] ? p->opt + 1 : NULL;
151-
return get_value(p, options, OPT_SHORT);
196+
return get_value(p, options, all_opts, OPT_SHORT);
152197
}
153198

154199
/*
@@ -177,6 +222,7 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
177222
static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
178223
const struct option *options)
179224
{
225+
const struct option *all_opts = options;
180226
const char *arg_end = strchr(arg, '=');
181227
const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
182228
int abbrev_flags = 0, ambiguous_flags = 0;
@@ -253,7 +299,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
253299
continue;
254300
p->opt = rest + 1;
255301
}
256-
return get_value(p, options, flags ^ opt_flags);
302+
return get_value(p, options, all_opts, flags ^ opt_flags);
257303
}
258304

259305
if (ambiguous_option)
@@ -265,18 +311,20 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,
265311
(abbrev_flags & OPT_UNSET) ? "no-" : "",
266312
abbrev_option->long_name);
267313
if (abbrev_option)
268-
return get_value(p, abbrev_option, abbrev_flags);
314+
return get_value(p, abbrev_option, all_opts, abbrev_flags);
269315
return -2;
270316
}
271317

272318
static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
273319
const struct option *options)
274320
{
321+
const struct option *all_opts = options;
322+
275323
for (; options->type != OPTION_END; options++) {
276324
if (!(options->flags & PARSE_OPT_NODASH))
277325
continue;
278326
if (options->short_name == arg[0] && arg[1] == '\0')
279-
return get_value(p, options, OPT_SHORT);
327+
return get_value(p, options, all_opts, OPT_SHORT);
280328
}
281329
return -2;
282330
}

‎parse-options.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ enum parse_opt_type {
1313
OPTION_COUNTUP,
1414
OPTION_SET_INT,
1515
OPTION_SET_PTR,
16+
OPTION_CMDMODE,
1617
/* options with arguments (usually) */
1718
OPTION_STRING,
1819
OPTION_INTEGER,
@@ -130,6 +131,8 @@ struct option {
130131
#define OPT_BOOL(s, l, v, h) OPT_SET_INT(s, l, v, h, 1)
131132
#define OPT_SET_PTR(s, l, v, h, p) { OPTION_SET_PTR, (s), (l), (v), NULL, \
132133
(h), PARSE_OPT_NOARG, NULL, (p) }
134+
#define OPT_CMDMODE(s, l, v, h, i) { OPTION_CMDMODE, (s), (l), (v), NULL, \
135+
(h), PARSE_OPT_NOARG|PARSE_OPT_NONEG, NULL, (i) }
133136
#define OPT_INTEGER(s, l, v, h) { OPTION_INTEGER, (s), (l), (v), N_("n"), (h) }
134137
#define OPT_STRING(s, l, v, a, h) { OPTION_STRING, (s), (l), (v), (a), (h) }
135138
#define OPT_STRING_LIST(s, l, v, a, h) \

0 commit comments

Comments
 (0)