Skip to content

Commit

Permalink
Merge branch 'ab/parse-options-cleanup'
Browse files Browse the repository at this point in the history 
Random changes to parse-options implementation.

* ab/parse-options-cleanup:
  parse-options: change OPT_{SHORT,UNSET} to an enum
  parse-options tests: test optname() output
  parse-options.[ch]: make opt{bug,name}() "static"
  commit-graph: stop using optname()
  parse-options.c: move optname() earlier in the file
  parse-options.h: make the "flags" in "struct option" an enum
  parse-options.c: use exhaustive "case" arms for "enum parse_opt_result"
  parse-options.[ch]: consistently use "enum parse_opt_result"
  parse-options.[ch]: consistently use "enum parse_opt_flags"
  parse-options.h: move PARSE_OPT_SHELL_EVAL between enums
  • Loading branch information
@gitster
gitster committed Oct 25, 2021
2 parents f3f157f + d342834 commit 65ca324
Show file tree
Hide file tree
Showing 6 changed files with 109 additions and 56 deletions.
3 changes: 3 additions & 0 deletions builtin/blame.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix)
PARSE_OPT_KEEP_DASHDASH | PARSE_OPT_KEEP_ARGV0);
for (;;) {
switch (parse_options_step(&ctx, options, blame_opt_usage)) {
case PARSE_OPT_NON_OPTION:
case PARSE_OPT_UNKNOWN:
break;
case PARSE_OPT_HELP:
case PARSE_OPT_ERROR:
exit(129);
Expand Down
4 changes: 2 additions & 2 deletions builtin/commit-graph.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -172,8 +172,8 @@ static int write_option_max_new_filters(const struct option *opt,
const char *s;
*to = strtol(arg, (char **)&s, 10);
if (*s)
return error(_("%s expects a numerical value"),
optname(opt, opt->flags));
return error(_("option `%s' expects a numerical value"),
"max-new-filters");
}
return 0;
}
Expand Down
3 changes: 3 additions & 0 deletions builtin/shortlog.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,9 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)

for (;;) {
switch (parse_options_step(&ctx, options, shortlog_usage)) {
case PARSE_OPT_NON_OPTION:
case PARSE_OPT_UNKNOWN:
break;
case PARSE_OPT_HELP:
case PARSE_OPT_ERROR:
exit(129);
Expand Down
87 changes: 49 additions & 38 deletions parse-options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@

static int disallow_abbreviated_options;

#define OPT_SHORT 1
#define OPT_UNSET 2
enum opt_parsed {
OPT_LONG = 0,
OPT_SHORT = 1<<0,
OPT_UNSET = 1<<1,
};

int optbug(const struct option *opt, const char *reason)
static int optbug(const struct option *opt, const char *reason)
{
if (opt->long_name) {
if (opt->short_name)
Expand All @@ -22,9 +25,26 @@ int optbug(const struct option *opt, const char *reason)
return error("BUG: switch '%c' %s", opt->short_name, reason);
}

static const char *optname(const struct option *opt, enum opt_parsed flags)
{
static struct strbuf sb = STRBUF_INIT;

strbuf_reset(&sb);
if (flags & OPT_SHORT)
strbuf_addf(&sb, "switch `%c'", opt->short_name);
else if (flags & OPT_UNSET)
strbuf_addf(&sb, "option `no-%s'", opt->long_name);
else if (flags == OPT_LONG)
strbuf_addf(&sb, "option `%s'", opt->long_name);
else
BUG("optname() got unknown flags %d", flags);

return sb.buf;
}

static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
const struct option *opt,
int flags, const char **arg)
enum opt_parsed flags, const char **arg)
{
if (p->opt) {
*arg = p->opt;
Expand All @@ -50,7 +70,7 @@ static void fix_filename(const char *prefix, const char **file)
static enum parse_opt_result opt_command_mode_error(
const struct option *opt,
const struct option *all_opts,
int flags)
enum opt_parsed flags)
{
const struct option *that;
struct strbuf that_name = STRBUF_INIT;
Expand Down Expand Up @@ -82,7 +102,7 @@ static enum parse_opt_result opt_command_mode_error(
static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
const struct option *opt,
const struct option *all_opts,
int flags)
enum opt_parsed flags)
{
const char *s, *arg;
const int unset = flags & OPT_UNSET;
Expand Down Expand Up @@ -298,11 +318,11 @@ static enum parse_opt_result parse_long_opt(
const struct option *all_opts = options;
const char *arg_end = strchrnul(arg, '=');
const struct option *abbrev_option = NULL, *ambiguous_option = NULL;
int abbrev_flags = 0, ambiguous_flags = 0;
enum opt_parsed abbrev_flags = OPT_LONG, ambiguous_flags = OPT_LONG;

for (; options->type != OPTION_END; options++) {
const char *rest, *long_name = options->long_name;
int flags = 0, opt_flags = 0;
enum opt_parsed flags = OPT_LONG, opt_flags = OPT_LONG;

if (!long_name)
continue;
Expand Down Expand Up @@ -481,7 +501,8 @@ static void parse_options_check(const struct option *opts)

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)
const struct option *options,
enum parse_opt_flags flags)
{
ctx->argc = argc;
ctx->argv = argv;
Expand All @@ -506,7 +527,8 @@ static void parse_options_start_1(struct parse_opt_ctx_t *ctx,

void parse_options_start(struct parse_opt_ctx_t *ctx,
int argc, const char **argv, const char *prefix,
const struct option *options, int flags)
const struct option *options,
enum parse_opt_flags flags)
{
memset(ctx, 0, sizeof(*ctx));
parse_options_start_1(ctx, argc, argv, prefix, options, flags);
Expand Down Expand Up @@ -697,13 +719,14 @@ static void free_preprocessed_options(struct option *options)
free(options);
}

static int usage_with_options_internal(struct parse_opt_ctx_t *,
const char * const *,
const struct option *, int, int);
static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t *,
const char * const *,
const struct option *,
int, int);

int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[])
enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[])
{
int internal_help = !(ctx->flags & PARSE_OPT_NO_INTERNAL_HELP);

Expand Down Expand Up @@ -837,9 +860,11 @@ int parse_options_end(struct parse_opt_ctx_t *ctx)
return ctx->cpidx + ctx->argc;
}

int parse_options(int argc, const char **argv, const char *prefix,
const struct option *options, const char * const usagestr[],
int flags)
enum parse_opt_result parse_options(int argc, const char **argv,
const char *prefix,
const struct option *options,
const char * const usagestr[],
enum parse_opt_flags flags)
{
struct parse_opt_ctx_t ctx;
struct option *real_options;
Expand All @@ -861,7 +886,7 @@ int parse_options(int argc, const char **argv, const char *prefix,
case PARSE_OPT_NON_OPTION:
case PARSE_OPT_DONE:
break;
default: /* PARSE_OPT_UNKNOWN */
case PARSE_OPT_UNKNOWN:
if (ctx.argv[0][1] == '-') {
error(_("unknown option `%s'"), ctx.argv[0] + 2);
} else if (isascii(*ctx.opt)) {
Expand Down Expand Up @@ -897,9 +922,10 @@ static int usage_argh(const struct option *opts, FILE *outfile)
#define USAGE_OPTS_WIDTH 24
#define USAGE_GAP 2

static int usage_with_options_internal(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
const struct option *opts, int full, int err)
static enum parse_opt_result usage_with_options_internal(struct parse_opt_ctx_t *ctx,
const char * const *usagestr,
const struct option *opts,
int full, int err)
{
FILE *outfile = err ? stderr : stdout;
int need_newline;
Expand Down Expand Up @@ -1052,18 +1078,3 @@ void NORETURN usage_msg_opt(const char *msg,
fprintf(stderr, "fatal: %s\n\n", msg);
usage_with_options(usagestr, options);
}

const char *optname(const struct option *opt, int flags)
{
static struct strbuf sb = STRBUF_INIT;

strbuf_reset(&sb);
if (flags & OPT_SHORT)
strbuf_addf(&sb, "switch `%c'", opt->short_name);
else if (flags & OPT_UNSET)
strbuf_addf(&sb, "option `no-%s'", opt->long_name);
else
strbuf_addf(&sb, "option `%s'", opt->long_name);

return sb.buf;
}
26 changes: 13 additions & 13 deletions parse-options.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ enum parse_opt_flags {
PARSE_OPT_KEEP_UNKNOWN = 1 << 3,
PARSE_OPT_NO_INTERNAL_HELP = 1 << 4,
PARSE_OPT_ONE_SHOT = 1 << 5,
PARSE_OPT_SHELL_EVAL = 1 << 6,
};

enum parse_opt_option_flags {
Expand All @@ -44,7 +45,6 @@ enum parse_opt_option_flags {
PARSE_OPT_NODASH = 1 << 5,
PARSE_OPT_LITERAL_ARGHELP = 1 << 6,
PARSE_OPT_FROM_ALIAS = 1 << 7,
PARSE_OPT_SHELL_EVAL = 1 << 8,
PARSE_OPT_NOCOMPLETE = 1 << 9,
PARSE_OPT_COMP_ARG = 1 << 10,
PARSE_OPT_CMDMODE = 1 << 11,
Expand Down Expand Up @@ -134,7 +134,7 @@ struct option {
const char *argh;
const char *help;

int flags;
enum parse_opt_option_flags flags;
parse_opt_cb *callback;
intptr_t defval;
parse_opt_ll_cb *ll_callback;
Expand Down Expand Up @@ -213,9 +213,11 @@ struct option {
* untouched and parse_options() returns the number of options
* processed.
*/
int parse_options(int argc, const char **argv, const char *prefix,
const struct option *options,
const char * const usagestr[], int flags);
enum parse_opt_result parse_options(int argc, const char **argv,
const char *prefix,
const struct option *options,
const char * const usagestr[],
enum parse_opt_flags flags);

NORETURN void usage_with_options(const char * const *usagestr,
const struct option *options);
Expand All @@ -224,9 +226,6 @@ NORETURN void usage_msg_opt(const char *msg,
const char * const *usagestr,
const struct option *options);

int optbug(const struct option *opt, const char *reason);
const char *optname(const struct option *opt, int flags);

/*
* Use these assertions for callbacks that expect to be called with NONEG and
* NOARG respectively, and do not otherwise handle the "unset" and "arg"
Expand Down Expand Up @@ -264,19 +263,20 @@ struct parse_opt_ctx_t {
const char **out;
int argc, cpidx, total;
const char *opt;
int flags;
enum parse_opt_flags 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,
int argc, const char **argv, const char *prefix,
const struct option *options, int flags);
const struct option *options,
enum parse_opt_flags flags);

int parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[]);
enum parse_opt_result parse_options_step(struct parse_opt_ctx_t *ctx,
const struct option *options,
const char * const usagestr[]);

int parse_options_end(struct parse_opt_ctx_t *ctx);

Expand Down
42 changes: 39 additions & 3 deletions t/t0040-parse-options.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,9 +168,45 @@ test_expect_success 'long options' '
'

test_expect_success 'missing required value' '
test_expect_code 129 test-tool parse-options -s &&
test_expect_code 129 test-tool parse-options --string &&
test_expect_code 129 test-tool parse-options --file
cat >expect <<-\EOF &&
error: switch `s'\'' requires a value
EOF
test_expect_code 129 test-tool parse-options -s 2>actual &&
test_cmp expect actual &&
cat >expect <<-\EOF &&
error: option `string'\'' requires a value
EOF
test_expect_code 129 test-tool parse-options --string 2>actual &&
test_cmp expect actual &&
cat >expect <<-\EOF &&
error: option `file'\'' requires a value
EOF
test_expect_code 129 test-tool parse-options --file 2>actual &&
test_cmp expect actual
'

test_expect_success 'superfluous value provided: boolean' '
cat >expect <<-\EOF &&
error: option `yes'\'' takes no value
EOF
test_expect_code 129 test-tool parse-options --yes=hi 2>actual &&
test_cmp expect actual &&
cat >expect <<-\EOF &&
error: option `no-yes'\'' takes no value
EOF
test_expect_code 129 test-tool parse-options --no-yes=hi 2>actual &&
test_cmp expect actual
'

test_expect_success 'superfluous value provided: cmdmode' '
cat >expect <<-\EOF &&
error: option `mode1'\'' takes no value
EOF
test_expect_code 129 test-tool parse-options --mode1=hi 2>actual &&
test_cmp expect actual
'

cat >expect <<\EOF
Expand Down

0 comments on commit 65ca324

Please sign in to comment.