diff options
| -rw-r--r-- | builtin/blame.c | 3 | ||||
| -rw-r--r-- | builtin/commit-graph.c | 4 | ||||
| -rw-r--r-- | builtin/shortlog.c | 3 | ||||
| -rw-r--r-- | parse-options.c | 87 | ||||
| -rw-r--r-- | parse-options.h | 26 | ||||
| -rwxr-xr-x | t/t0040-parse-options.sh | 42 |
6 files changed, 109 insertions, 56 deletions
diff --git a/builtin/blame.c b/builtin/blame.c index 1c31a99640..f9ee3f8c68 100644 --- a/builtin/blame.c +++ b/builtin/blame.c @@ -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); diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 3c3de3a156..ab8e5cd59a 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -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; } diff --git a/builtin/shortlog.c b/builtin/shortlog.c index 3e7ab1ca82..e7f7af5de3 100644 --- a/builtin/shortlog.c +++ b/builtin/shortlog.c @@ -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); diff --git a/parse-options.c b/parse-options.c index 6e0535bdaa..9a0484c883 100644 --- a/parse-options.c +++ b/parse-options.c @@ -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) @@ -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; @@ -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; @@ -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; @@ -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; @@ -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; @@ -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); @@ -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); @@ -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; @@ -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)) { @@ -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; @@ -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; -} diff --git a/parse-options.h b/parse-options.h index 13405472ee..bdea052c39 100644 --- a/parse-options.h +++ b/parse-options.h @@ -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 { @@ -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, @@ -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; @@ -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); @@ -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" @@ -264,7 +263,7 @@ 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; @@ -272,11 +271,12 @@ struct parse_opt_ctx_t { 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); diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh index da310ed29b..d6f391a497 100755 --- a/t/t0040-parse-options.sh +++ b/t/t0040-parse-options.sh @@ -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 |