Skip to content

Commit 5c38742

Browse files
pcloudsgitster
pclouds
authored and
gitster
committed
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 ccdd3da ("clone: Add the --recurse-submodules option as alias for --recursive", 2010-11-04). Since bb62e0a ("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>
1 parent 83232e3 commit 5c38742

File tree

5 files changed

+164
-10
lines changed

5 files changed

+164
-10
lines changed

‎builtin/clone.c‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,7 @@ static struct option builtin_clone_options[] = {
9898
N_("don't use local hardlinks, always copy")),
9999
OPT_BOOL('s', "shared", &option_shared,
100100
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"),
105102
{ OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
106103
N_("pathspec"), N_("initialize submodules in the clone"),
107104
PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },

‎parse-options.c‎

Lines changed: 137 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,35 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
261261
return PARSE_OPT_UNKNOWN;
262262
}
263263

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+
264293
static enum parse_opt_result parse_long_opt(
265294
struct parse_opt_ctx_t *p, const char *arg,
266295
const struct option *options)
@@ -298,7 +327,8 @@ static enum parse_opt_result parse_long_opt(
298327
if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
299328
!strncmp(long_name, arg, arg_end - arg)) {
300329
is_abbreviated:
301-
if (abbrev_option) {
330+
if (abbrev_option &&
331+
!is_alias(p, abbrev_option, options)) {
302332
/*
303333
* If this is abbreviated, it is
304334
* ambiguous. So when there is no
@@ -447,6 +477,10 @@ static void parse_options_check(const struct option *opts)
447477
if (opts->callback)
448478
BUG("OPTION_LOWLEVEL_CALLBACK needs no high level callback");
449479
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.");
450484
default:
451485
; /* ok. (usually accepts an argument) */
452486
}
@@ -458,11 +492,10 @@ static void parse_options_check(const struct option *opts)
458492
exit(128);
459493
}
460494

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)
464498
{
465-
memset(ctx, 0, sizeof(*ctx));
466499
ctx->argc = argc;
467500
ctx->argv = argv;
468501
if (!(flags & PARSE_OPT_ONE_SHOT)) {
@@ -484,6 +517,14 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
484517
parse_options_check(options);
485518
}
486519

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+
487528
static void show_negated_gitcomp(const struct option *opts, int nr_noopts)
488529
{
489530
int printed_dashdash = 0;
@@ -575,6 +616,83 @@ static int show_gitcomp(const struct option *opts)
575616
return PARSE_OPT_COMPLETE;
576617
}
577618

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+
578696
static int usage_with_options_internal(struct parse_opt_ctx_t *,
579697
const char * const *,
580698
const struct option *, int, int);
@@ -714,11 +832,16 @@ int parse_options(int argc, const char **argv, const char *prefix,
714832
int flags)
715833
{
716834
struct parse_opt_ctx_t ctx;
835+
struct option *real_options;
717836

718837
disallow_abbreviated_options =
719838
git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0);
720839

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);
722845
switch (parse_options_step(&ctx, options, usagestr)) {
723846
case PARSE_OPT_HELP:
724847
case PARSE_OPT_ERROR:
@@ -741,6 +864,8 @@ int parse_options(int argc, const char **argv, const char *prefix,
741864
}
742865

743866
precompose_argv(argc, argv);
867+
free(real_options);
868+
free(ctx.alias_groups);
744869
return parse_options_end(&ctx);
745870
}
746871

@@ -835,6 +960,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
835960
fputc('\n', outfile);
836961
pad = USAGE_OPTS_WIDTH;
837962
}
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+
}
838969
fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help));
839970
}
840971
fputc('\n', outfile);

‎parse-options.h‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ enum parse_opt_type {
77
OPTION_ARGUMENT,
88
OPTION_GROUP,
99
OPTION_NUMBER,
10+
OPTION_ALIAS,
1011
/* options with no arguments */
1112
OPTION_BIT,
1213
OPTION_NEGBIT,
@@ -183,6 +184,9 @@ struct option {
183184
N_("no-op (backward compatibility)"), \
184185
PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb }
185186

187+
#define OPT_ALIAS(s, l, source_long_name) \
188+
{ OPTION_ALIAS, (s), (l), (source_long_name) }
189+
186190
/*
187191
* parse_options() will filter out the processed options and leave the
188192
* non-option arguments in argv[]. argv0 is assumed program name and
@@ -258,6 +262,8 @@ struct parse_opt_ctx_t {
258262
const char *opt;
259263
int flags;
260264
const char *prefix;
265+
const char **alias_groups; /* must be in groups of 3 elements! */
266+
struct option *updated_options;
261267
};
262268

263269
void parse_options_start(struct parse_opt_ctx_t *ctx,

‎t/helper/test-parse-options.c‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,9 @@ int cmd__parse_options(int argc, const char **argv)
149149
OPT_CALLBACK(0, "expect", &expect, "string",
150150
"expected output in the variable dump",
151151
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"),
152155
OPT_END(),
153156
};
154157
int i;

‎t/t0040-parse-options.sh‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,12 @@ Standard options
4848
-q, --quiet be quiet
4949
--expect <string> expected output in the variable dump
5050
51+
Alias
52+
-A, --alias-source <string>
53+
get a string
54+
-Z, --alias-target <string>
55+
get a string
56+
5157
EOF
5258

5359
test_expect_success 'test help' '
@@ -224,6 +230,17 @@ test_expect_success 'non ambiguous option (after two options it abbreviates)' '
224230
test-tool parse-options --expect="string: 123" --st 123
225231
'
226232

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+
227244
cat >typo.err <<\EOF
228245
error: did you mean `--boolean` (with two dashes ?)
229246
EOF

0 commit comments

Comments
 (0)