kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

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>
maint
Nguyễn Thái Ngọc Duy 2019-04-29 17:05:25 +07:00 committed by Junio C Hamano
parent 83232e3864
commit 5c387428f1
5 changed files with 164 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
builtin/clone.c
View File

@ -98,10 +98,7 @@ static struct option builtin_clone_options[] = {
N_("don't use local hardlinks, always copy")),
OPT_BOOL('s', "shared", &option_shared,
N_("setup as shared repository")),
{ OPTION_CALLBACK, 0, "recursive", &option_recurse_submodules,
N_("pathspec"), N_("initialize submodules in the clone"),
PARSE_OPT_OPTARG | PARSE_OPT_HIDDEN, recurse_submodules_cb,
(intptr_t)"." },
OPT_ALIAS(0, "recursive", "recurse-submodules"),
{ OPTION_CALLBACK, 0, "recurse-submodules", &option_recurse_submodules,
N_("pathspec"), N_("initialize submodules in the clone"),
PARSE_OPT_OPTARG, recurse_submodules_cb, (intptr_t)"." },

143
parse-options.c
View File

@ -261,6 +261,35 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
return PARSE_OPT_UNKNOWN;
}

static int has_string(const char *it, const char **array)
{
while (*array)
if (!strcmp(it, *(array++)))
return 1;
return 0;
}

static int is_alias(struct parse_opt_ctx_t *ctx,
const struct option *one_opt,
const struct option *another_opt)
{
const char **group;

if (!ctx->alias_groups)
return 0;

if (!one_opt->long_name || !another_opt->long_name)
return 0;

for (group = ctx->alias_groups; *group; group += 3) {
/* it and other are from the same family? */
if (has_string(one_opt->long_name, group) &&
has_string(another_opt->long_name, group))
return 1;
}
return 0;
}

static enum parse_opt_result parse_long_opt(
struct parse_opt_ctx_t *p, const char *arg,
const struct option *options)
@ -298,7 +327,8 @@ again:
if (!(p->flags & PARSE_OPT_KEEP_UNKNOWN) &&
!strncmp(long_name, arg, arg_end - arg)) {
is_abbreviated:
if (abbrev_option) {
if (abbrev_option &&
!is_alias(p, abbrev_option, options)) {
/*
* If this is abbreviated, it is
* ambiguous. So when there is no
@ -447,6 +477,10 @@ static void parse_options_check(const struct option *opts)
if (opts->callback)
BUG("OPTION_LOWLEVEL_CALLBACK needs no high level callback");
break;
case OPTION_ALIAS:
BUG("OPT_ALIAS() should not remain at this point. "
"Are you using parse_options_step() directly?\n"
"That case is not supported yet.");
default:
; /* ok. (usually accepts an argument) */
}
@ -458,11 +492,10 @@ static void parse_options_check(const struct option *opts)
exit(128);
}

void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, const char *prefix,
const struct option *options, int flags)
static void parse_options_start_1(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, const char *prefix,
const struct option *options, int flags)
{
memset(ctx, 0, sizeof(*ctx));
ctx->argc = argc;
ctx->argv = argv;
if (!(flags & PARSE_OPT_ONE_SHOT)) {
@ -484,6 +517,14 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
parse_options_check(options);
}

void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, const char *prefix,
const struct option *options, int flags)
{
memset(ctx, 0, sizeof(*ctx));
parse_options_start_1(ctx, argc, argv, prefix, options, flags);
}

static void show_negated_gitcomp(const struct option *opts, int nr_noopts)
{
int printed_dashdash = 0;
@ -575,6 +616,83 @@ static int show_gitcomp(const struct option *opts)
return PARSE_OPT_COMPLETE;
}

/*
* Scan and may produce a new option[] array, which should be used
* instead of the original 'options'.
*
* Right now this is only used to preprocess and substitue
* OPTION_ALIAS.
*/
static struct option *preprocess_options(struct parse_opt_ctx_t *ctx,
const struct option *options)
{
struct option *newopt;
int i, nr, alias;
int nr_aliases = 0;

for (nr = 0; options[nr].type != OPTION_END; nr++) {
if (options[nr].type == OPTION_ALIAS)
nr_aliases++;
}

if (!nr_aliases)
return NULL;

ALLOC_ARRAY(newopt, nr + 1);
COPY_ARRAY(newopt, options, nr + 1);

/* each alias has two string pointers and NULL */
CALLOC_ARRAY(ctx->alias_groups, 3 * (nr_aliases + 1));

for (alias = 0, i = 0; i < nr; i++) {
int short_name;
const char *long_name;
const char *source;
int j;

if (newopt[i].type != OPTION_ALIAS)
continue;

short_name = newopt[i].short_name;
long_name = newopt[i].long_name;
source = newopt[i].value;

if (!long_name)
BUG("An alias must have long option name");

for (j = 0; j < nr; j++) {
const char *name = options[j].long_name;

if (!name || strcmp(name, source))
continue;

if (options[j].type == OPTION_ALIAS)
BUG("No please. Nested aliases are not supported.");

/*
* NEEDSWORK: this is a bit inconsistent because
* usage_with_options() on the original options[] will print
* help string as "alias of %s" but "git cmd -h" will
* print the original help string.
*/
memcpy(newopt + i, options + j, sizeof(*newopt));
newopt[i].short_name = short_name;
newopt[i].long_name = long_name;
break;
}

if (j == nr)
BUG("could not find source option '%s' of alias '%s'",
source, newopt[i].long_name);
ctx->alias_groups[alias * 3 + 0] = newopt[i].long_name;
ctx->alias_groups[alias * 3 + 1] = options[j].long_name;
ctx->alias_groups[alias * 3 + 2] = NULL;
alias++;
}

return newopt;
}

static int usage_with_options_internal(struct parse_opt_ctx_t *,
const char * const *,
const struct option *, int, int);
@ -714,11 +832,16 @@ int parse_options(int argc, const char **argv, const char *prefix,
int flags)
{
struct parse_opt_ctx_t ctx;
struct option *real_options;

disallow_abbreviated_options =
git_env_bool("GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS", 0);

parse_options_start(&ctx, argc, argv, prefix, options, flags);
memset(&ctx, 0, sizeof(ctx));
real_options = preprocess_options(&ctx, options);
if (real_options)
options = real_options;
parse_options_start_1(&ctx, argc, argv, prefix, options, flags);
switch (parse_options_step(&ctx, options, usagestr)) {
case PARSE_OPT_HELP:
case PARSE_OPT_ERROR:
@ -741,6 +864,8 @@ int parse_options(int argc, const char **argv, const char *prefix,
}

precompose_argv(argc, argv);
free(real_options);
free(ctx.alias_groups);
return parse_options_end(&ctx);
}

@ -835,6 +960,12 @@ static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
fputc('\n', outfile);
pad = USAGE_OPTS_WIDTH;
}
if (opts->type == OPTION_ALIAS) {
fprintf(outfile, "%*s", pad + USAGE_GAP, "");
fprintf_ln(outfile, _("alias of --%s"),
(const char *)opts->value);
continue;
}
fprintf(outfile, "%*s%s\n", pad + USAGE_GAP, "", _(opts->help));
}
fputc('\n', outfile);

6
parse-options.h
View File

@ -7,6 +7,7 @@ enum parse_opt_type {
OPTION_ARGUMENT,
OPTION_GROUP,
OPTION_NUMBER,
OPTION_ALIAS,
/* options with no arguments */
OPTION_BIT,
OPTION_NEGBIT,
@ -183,6 +184,9 @@ struct option {
N_("no-op (backward compatibility)"), \
PARSE_OPT_HIDDEN | PARSE_OPT_NOARG, parse_opt_noop_cb }

#define OPT_ALIAS(s, l, source_long_name) \
{ OPTION_ALIAS, (s), (l), (source_long_name) }

/*
* parse_options() will filter out the processed options and leave the
* non-option arguments in argv[]. argv0 is assumed program name and
@ -258,6 +262,8 @@ struct parse_opt_ctx_t {
const char *opt;
int flags;
const char *prefix;
const char **alias_groups; /* must be in groups of 3 elements! */
struct option *updated_options;
};

void parse_options_start(struct parse_opt_ctx_t *ctx,

3
t/helper/test-parse-options.c
View File

@ -149,6 +149,9 @@ int cmd__parse_options(int argc, const char **argv)
OPT_CALLBACK(0, "expect", &expect, "string",
"expected output in the variable dump",
collect_expect),
OPT_GROUP("Alias"),
OPT_STRING('A', "alias-source", &string, "string", "get a string"),
OPT_ALIAS('Z', "alias-target", "alias-source"),
OPT_END(),
};
int i;

17
t/t0040-parse-options.sh
View File

@ -48,6 +48,12 @@ Standard options
-q, --quiet be quiet
--expect <string> expected output in the variable dump

Alias
-A, --alias-source <string>
get a string
-Z, --alias-target <string>
get a string

EOF

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

test_expect_success 'Alias options do not contribute to abbreviation' '
test-tool parse-options --alias-source 123 >output &&
grep "^string: 123" output &&
test-tool parse-options --alias-target 123 >output &&
grep "^string: 123" output &&
test_must_fail test-tool parse-options --alias &&
GIT_TEST_DISALLOW_ABBREVIATED_OPTIONS=false \
test-tool parse-options --alias 123 >output &&
grep "^string: 123" output
'

cat >typo.err <<\EOF
error: did you mean `--boolean` (with two dashes ?)
EOF