parse-options: don't emit "ambiguous option" for aliases
Change the option parsing machinery so that e.g. "clone --recurs ..." doesn't error out because "clone" understands both "--recursive" and "--recurse-submodules" to mean the same thing. Initially "clone" just understood --recursive until the --recurses-submodules alias was added in ccdd3da652 ("clone: Add the --recurse-submodules option as alias for --recursive", 2010-11-04). Since bb62e0a99f ("clone: teach --recurse-submodules to optionally take a pathspec", 2017-03-17) the longer form has been promoted to the default. But due to the way the options parsing machinery works this resulted in the rather absurd situation of: $ git clone --recurs [...] error: ambiguous option: recurs (could be --recursive or --recurse-submodules) Add OPT_ALIAS() to express this link between two or more options and use it in git-clone. Multiple aliases of an option could be written as OPT_ALIAS(0, "alias1", "original-name"), OPT_ALIAS(0, "alias2", "original-name"), ... The current implementation is not exactly optimal in this case. But we can optimize it when it becomes a problem. So far we don't even have two aliases of any option. A big chunk of code is actually from Junio C Hamano. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Nguyễn Thái Ngọc Duy committed
Apr 29, 2019 at 17:05 UTC
5c387428f10c27c24d3adb890cd466e2300518fa
5 files changed
+164
-10
builtin/clone.c
+1
-4
@@ -98,10 +98,7 @@ static struct option builtin_clone_options[] = {
98
N_("don't use local hardlinks, always copy")),
99
OPT_BOOL('s', "shared", &option_shared,
100
N_("setup as shared repository")),
101
- { OPTION_CALLBACK, 0, "recursive", &option_recurse_submodules,
102
- N_("pathspec"), N_("initialize submodules in the clone"),
103
- PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, recurse_submodules_cb,
104
- (intptr_t)"." },
101
+ OPT_ALIAS(0, "recursive", "recurse-submodules"),
102
{ OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
103
N_("pathspec"), N_("initialize submodules in the clone"),
104
PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },
parse-options.c
+137
-6
@@ -261,6 +261,35 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
261
return PARSE_OPT_UNKNOWN;
262
}
263
264
+static int has_string(const char *it, const char **array)
265
+{
266
+ while (*array)
267
+ if (!strcmp(it, *(array++)))
268
+ return 1;
269
+ return 0;
270
+}
271
+
272
+static int is_alias(struct parse_opt_ctx_t *ctx,
273
+ const struct option *one_opt,
274
+ const struct option *another_opt)
275
+{
276
+ const char **group;
277
+
278
+ if (!ctx->alias_groups)
279
+ return 0;
280
+
281
+ if (!one_opt->long_name || !another_opt->long_name)
282
+ return 0;
283
+
284
+ for (group = ctx->alias_groups; *group; group += 3) {
285
+ /* it and other are from the same family? */
286
+ if (has_string(one_opt->long_name, group) &&
287
+ has_string(another_opt->long_name, group))
288
+ return 1;
289
+ }
290
+ return 0;
291
+}
292
+
293
static enum parse_opt_result parse_long_opt(
294
struct parse_opt_ctx_t *p, const char *arg,
295
const struct option *options)
@@ -298,7 +327,8 @@ again:
327
if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
328
!strncmp(long_name, arg, arg_end - arg)) {
329
is_abbreviated:
301
- if (abbrev_option) {
330
+ if (abbrev_option &&
331
+ !is_alias(p, abbrev_option, options)) {
332
/*
333
* If this is abbreviated, it is
334
* ambiguous. So when there is no
@@ -447,6 +477,10 @@ static void parse_options_check(const struct option *opts)
477
if (opts->callback)
478
BUG("OPTION_LOWLEVEL_CALLBACK needs no high level callback");
479
break;
480
+ case OPTION_ALIAS:
481
+ BUG("OPT_ALIAS() should not remain at this point. "
482
+ "Are you using parse_options_step() directly?\n"
483
+ "That case is not supported yet.");
484
default:
485
; /* ok. (usually accepts an argument) */
486
}
@@ -458,11 +492,10 @@ static void parse_options_check(const struct option *opts)
492
exit(128);
493
}
494
461
-void parse_options_start(struct parse_opt_ctx_t *ctx,
462
- int argc, const char **argv, const char *prefix,
463
- const struct option *options, int flags)
495
+static void parse_options_start_1(struct parse_opt_ctx_t *ctx,
496
+ int argc, const char **argv, const char *prefix,
497
+ const struct option *options, int flags)
498
{
465
- memset(ctx, 0, sizeof(*ctx));
499
ctx->argc = argc;
500
ctx->argv = argv;
501
if (!(flags & PARSE_OPT_ONE_SHOT)) {
@@ -484,6 +517,14 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
517
parse_options_check(options);
518
}
519
520
+void parse_options_start(struct parse_opt_ctx_t *ctx,
521
+ int argc, const char **argv, const char *prefix,
522
+ const struct option *options, int flags)
523
+{
524
+ memset(ctx, 0, sizeof(*ctx));
525
+ parse_options_start_1(ctx, argc, argv, prefix, options, flags);
526
+}
527
+
528
static void show_negated_gitcomp(const struct option *opts, int nr_noopts)
529
{
530
int printed_dashdash = 0;
@@ -575,6 +616,83 @@ static int show_gitcomp(const struct option *opts)
616
return PARSE_OPT_COMPLETE;
617
}
618
619
+/*
620
+ * Scan and may produce a new option[] array, which should be used
621
+ * instead of the original 'options'.
622
+ *
623
+ * Right now this is only used to preprocess and substitue
624
+ * OPTION_ALIAS.
625
+ */
626
+static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
627
+ const struct option *options)
628
+{
629
+ struct option *newopt;
630
+ int i, nr, alias;
631
+ int nr_aliases = 0;
632
+
633
+ for (nr = 0; options[nr].type != OPTION_END; nr++) {
634
+ if (options[nr].type == OPTION_ALIAS)
635
+ nr_aliases++;
636
+ }
637
+
638
+ if (!nr_aliases)
639
+ return NULL;
640
+
641
+ ALLOC_ARRAY(newopt, nr + 1);
642
+ COPY_ARRAY(newopt, options, nr + 1);
643
+
644
+ /* each alias has two string pointers and NULL */
645
+ CALLOC_ARRAY(ctx->alias_groups, 3 * (nr_aliases + 1));
646
+
647
+ for (alias = 0, i = 0; i < nr; i++) {
648
+ int short_name;
649
+ const char *long_name;
650
+ const char *source;
651
+ int j;
652
+
653
+ if (newopt[i].type != OPTION_ALIAS)
654
+ continue;
655
+
656
+ short_name = newopt[i].short_name;
657
+ long_name = newopt[i].long_name;
658
+ source = newopt[i].value;
659
+
660
+ if (!long_name)
661
+ BUG("An alias must have long option name");
662
+
663
+ for (j = 0; j < nr; j++) {
664
+ const char *name = options[j].long_name;
665
+
666
+ if (!name || strcmp(name, source))
667
+ continue;
668
+
669
+ if (options[j].type == OPTION_ALIAS)
670
+ BUG("No please. Nested aliases are not supported.");
671
+
672
+ /*
673
+ * NEEDSWORK: this is a bit inconsistent because
674
+ * usage_with_options() on the original options[] will print
675
+ * help string as "alias of %s" but "git cmd -h" will
676
+ * print the original help string.
677
+ */
678
+ memcpy(newopt + i, options + j, sizeof(*newopt));
679
+ newopt[i].short_name = short_name;
680
+ newopt[i].long_name = long_name;
681
+ break;
682
+ }
683
+
684
+ if (j == nr)
685
+ BUG("could not find source option '%s' of alias '%s'",
686
+ source, newopt[i].long_name);
687
+ ctx->alias_groups[alias * 3 + 0] = newopt[i].long_name;
688
+ ctx->alias_groups[alias * 3 + 1] = options[j].long_name;
689
+ ctx->alias_groups[alias * 3 + 2] = NULL;
690
+ alias++;
691
+ }
692
+
693
+ return newopt;
694
+}
695
+
696
static int usage_with_options_internal(struct parse_opt_ctx_t *,
697
const char * const *,
698
const struct option *, int, int);
@@ -714,11 +832,16 @@ int parse_options(int argc, const char **argv, const char *prefix,
832
int flags)
833
{
834
struct parse_opt_ctx_t ctx;
835
+ struct option *real_options;
836
837
disallow_abbreviated_options =
838
git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0);
839
721
- parse_options_start(&ctx, argc, argv, prefix, options, flags);
840
+ memset(&ctx, 0, sizeof(ctx));
841
+ real_options = preprocess_options(&ctx, options);
842
+ if (real_options)
843
+ options = real_options;
844
+ parse_options_start_1(&ctx, argc, argv, prefix, options, flags);
845
switch (parse_options_step(&ctx, options, usagestr)) {
846
case PARSE_OPT_HELP:
847
case PARSE_OPT_ERROR:
@@ -741,6 +864,8 @@ int parse_options(int argc, const char **argv, const char *prefix,
864
}
865
866
precompose_argv(argc, argv);
867
+ free(real_options);
868
+ free(ctx.alias_groups);
869
return parse_options_end(&ctx);
870
}
871
@@ -835,6 +960,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
960
fputc('\n', outfile);
961
pad = USAGE_OPTS_WIDTH;
962
}
963
+ if (opts->type == OPTION_ALIAS) {
964
+ fprintf(outfile, "%*s", pad + USAGE_GAP, "");
965
+ fprintf_ln(outfile, _("alias of --%s"),
966
+ (const char *)opts->value);
967
+ continue;
968
+ }
969
fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help));
970
}
971
fputc('\n', outfile);
parse-options.h
+6
@@ -7,6 +7,7 @@ enum parse_opt_type {
7
OPTION_ARGUMENT,
8
OPTION_GROUP,
9
OPTION_NUMBER,
10
+ OPTION_ALIAS,
11
/* options with no arguments */
12
OPTION_BIT,
13
OPTION_NEGBIT,
@@ -183,6 +184,9 @@ struct option {
184
N_("no-op (backward compatibility)"), \
185
PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb }
186
187
+#define OPT_ALIAS(s, l, source_long_name) \
188
+ { OPTION_ALIAS, (s), (l), (source_long_name) }
189
+
190
/*
191
* parse_options() will filter out the processed options and leave the
192
* non-option arguments in argv[]. argv0 is assumed program name and
@@ -258,6 +262,8 @@ struct parse_opt_ctx_t {
262
const char *opt;
263
int flags;
264
const char *prefix;
265
+ const char **alias_groups; /* must be in groups of 3 elements! */
266
+ struct option *updated_options;
267
};
268
269
void parse_options_start(struct parse_opt_ctx_t *ctx,
t/helper/test-parse-options.c
+3
@@ -149,6 +149,9 @@ int cmd__parse_options(int argc, const char **argv)
149
OPT_CALLBACK(0, "expect", &expect, "string",
150
"expected output in the variable dump",
151
collect_expect),
152
+ OPT_GROUP("Alias"),
153
+ OPT_STRING('A', "alias-source", &string, "string", "get a string"),
154
+ OPT_ALIAS('Z', "alias-target", "alias-source"),
155
OPT_END(),
156
};
157
int i;
t/t0040-parse-options.sh
+17
@@ -48,6 +48,12 @@ Standard options
48
-q, --quiet be quiet
49
--expect <string> expected output in the variable dump
50
51
+Alias
52
+ -A, --alias-source <string>
53
+ get a string
54
+ -Z, --alias-target <string>
55
+ get a string
56
+
57
EOF
58
59
test_expect_success 'test help' '
@@ -224,6 +230,17 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
230
test-tool parse-options --expect="string: 123" --st 123
231
'
232
233
+test_expect_success 'Alias options do not contribute to abbreviation' '
234
+ test-tool parse-options --alias-source 123 >output &&
235
+ grep "^string: 123" output &&
236
+ test-tool parse-options --alias-target 123 >output &&
237
+ grep "^string: 123" output &&
238
+ test_must_fail test-tool parse-options --alias &&
239
+ GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
240
+ test-tool parse-options --alias 123 >output &&
241
+ grep "^string: 123" output
242
+'
243
+
244
cat >typo.err <<\EOF
245
error: did you mean `--boolean` (with two dashes ?)
246
EOF