parse-options: avoid magic return codes

Give names to these magic negative numbers. Make parse_opt_ll_cb
return an enum to make clear it can actually control parse_options()
with different return values (parse_opt_cb can too, but nobody needs
it).

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Nguyễn Thái Ngọc Duy 2019-01-27 07:35:27 +07:00 committed by Junio C Hamano
parent bf3ff338a2
commit f41179f16b
5 changed files with 68 additions and 46 deletions

View File

@ -112,8 +112,9 @@ static int option_parse_message(const struct option *opt,
return 0; return 0;
} }
static int option_read_message(struct parse_opt_ctx_t *ctx, static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx,
const struct option *opt, int unset) const struct option *opt,
int unset)
{ {
struct strbuf *buf = opt->value; struct strbuf *buf = opt->value;
const char *arg; const char *arg;

View File

@ -847,8 +847,8 @@ static int parse_new_style_cacheinfo(const char *arg,
return 0; return 0;
} }
static int cacheinfo_callback(struct parse_opt_ctx_t *ctx, static enum parse_opt_result cacheinfo_callback(
const struct option *opt, int unset) struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{ {
struct object_id oid; struct object_id oid;
unsigned int mode; unsigned int mode;
@ -873,8 +873,8 @@ static int cacheinfo_callback(struct parse_opt_ctx_t *ctx,
return 0; return 0;
} }
static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx, static enum parse_opt_result stdin_cacheinfo_callback(
const struct option *opt, int unset) struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{ {
int *nul_term_line = opt->value; int *nul_term_line = opt->value;
@ -887,8 +887,8 @@ static int stdin_cacheinfo_callback(struct parse_opt_ctx_t *ctx,
return 0; return 0;
} }
static int stdin_callback(struct parse_opt_ctx_t *ctx, static enum parse_opt_result stdin_callback(
const struct option *opt, int unset) struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{ {
int *read_from_stdin = opt->value; int *read_from_stdin = opt->value;
@ -900,8 +900,8 @@ static int stdin_callback(struct parse_opt_ctx_t *ctx,
return 0; return 0;
} }
static int unresolve_callback(struct parse_opt_ctx_t *ctx, static enum parse_opt_result unresolve_callback(
const struct option *opt, int unset) struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{ {
int *has_errors = opt->value; int *has_errors = opt->value;
const char *prefix = startup_info->prefix; const char *prefix = startup_info->prefix;
@ -919,8 +919,8 @@ static int unresolve_callback(struct parse_opt_ctx_t *ctx,
return 0; return 0;
} }
static int reupdate_callback(struct parse_opt_ctx_t *ctx, static enum parse_opt_result reupdate_callback(
const struct option *opt, int unset) struct parse_opt_ctx_t *ctx, const struct option *opt, int unset)
{ {
int *has_errors = opt->value; int *has_errors = opt->value;
const char *prefix = startup_info->prefix; const char *prefix = startup_info->prefix;

View File

@ -170,10 +170,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset)
* "-h" output even if it's not being handled directly by * "-h" output even if it's not being handled directly by
* parse_options(). * parse_options().
*/ */
int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx,
const struct option *opt, int unset) const struct option *opt, int unset)
{ {
return -2; return PARSE_OPT_UNKNOWN;
} }
/** /**

View File

@ -20,8 +20,9 @@ int optbug(const struct option *opt, const char *reason)
return error("BUG: switch '%c' %s", opt->short_name, reason); return error("BUG: switch '%c' %s", opt->short_name, reason);
} }
static int get_arg(struct parse_opt_ctx_t *p, const struct option *opt, static enum parse_opt_result get_arg(struct parse_opt_ctx_t *p,
int flags, const char **arg) const struct option *opt,
int flags, const char **arg)
{ {
if (p->opt) { if (p->opt) {
*arg = p->opt; *arg = p->opt;
@ -44,9 +45,10 @@ static void fix_filename(const char *prefix, const char **file)
*file = prefix_filename(prefix, *file); *file = prefix_filename(prefix, *file);
} }
static int opt_command_mode_error(const struct option *opt, static enum parse_opt_result opt_command_mode_error(
const struct option *all_opts, const struct option *opt,
int flags) const struct option *all_opts,
int flags)
{ {
const struct option *that; const struct option *that;
struct strbuf that_name = STRBUF_INIT; struct strbuf that_name = STRBUF_INIT;
@ -69,16 +71,16 @@ static int opt_command_mode_error(const struct option *opt,
error(_("%s is incompatible with %s"), error(_("%s is incompatible with %s"),
optname(opt, flags), that_name.buf); optname(opt, flags), that_name.buf);
strbuf_release(&that_name); strbuf_release(&that_name);
return -1; return PARSE_OPT_ERROR;
} }
return error(_("%s : incompatible with something else"), return error(_("%s : incompatible with something else"),
optname(opt, flags)); optname(opt, flags));
} }
static int get_value(struct parse_opt_ctx_t *p, static enum parse_opt_result get_value(struct parse_opt_ctx_t *p,
const struct option *opt, const struct option *opt,
const struct option *all_opts, const struct option *all_opts,
int flags) int flags)
{ {
const char *s, *arg; const char *s, *arg;
const int unset = flags & OPT_UNSET; const int unset = flags & OPT_UNSET;
@ -208,7 +210,8 @@ static int get_value(struct parse_opt_ctx_t *p,
} }
} }
static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *options) static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p,
const struct option *options)
{ {
const struct option *all_opts = options; const struct option *all_opts = options;
const struct option *numopt = NULL; const struct option *numopt = NULL;
@ -239,11 +242,12 @@ static int parse_short_opt(struct parse_opt_ctx_t *p, const struct option *optio
free(arg); free(arg);
return rc; return rc;
} }
return -2; return PARSE_OPT_UNKNOWN;
} }
static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg, static enum parse_opt_result parse_long_opt(
const struct option *options) struct parse_opt_ctx_t *p, const char *arg,
const struct option *options)
{ {
const struct option *all_opts = options; const struct option *all_opts = options;
const char *arg_end = strchrnul(arg, '='); const char *arg_end = strchrnul(arg, '=');
@ -269,7 +273,7 @@ again:
if (*rest) if (*rest)
continue; continue;
p->out[p->cpidx++] = arg - 2; p->out[p->cpidx++] = arg - 2;
return 0; return PARSE_OPT_DONE;
} }
if (!rest) { if (!rest) {
/* abbreviated? */ /* abbreviated? */
@ -334,11 +338,11 @@ is_abbreviated:
ambiguous_option->long_name, ambiguous_option->long_name,
(abbrev_flags & OPT_UNSET) ? "no-" : "", (abbrev_flags & OPT_UNSET) ? "no-" : "",
abbrev_option->long_name); abbrev_option->long_name);
return -3; return PARSE_OPT_HELP;
} }
if (abbrev_option) if (abbrev_option)
return get_value(p, abbrev_option, all_opts, abbrev_flags); return get_value(p, abbrev_option, all_opts, abbrev_flags);
return -2; return PARSE_OPT_UNKNOWN;
} }
static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg, static int parse_nodash_opt(struct parse_opt_ctx_t *p, const char *arg,
@ -590,22 +594,28 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (arg[1] != '-') { if (arg[1] != '-') {
ctx->opt = arg + 1; ctx->opt = arg + 1;
switch (parse_short_opt(ctx, options)) { switch (parse_short_opt(ctx, options)) {
case -1: case PARSE_OPT_ERROR:
return PARSE_OPT_ERROR; return PARSE_OPT_ERROR;
case -2: case PARSE_OPT_UNKNOWN:
if (ctx->opt) if (ctx->opt)
check_typos(arg + 1, options); check_typos(arg + 1, options);
if (internal_help && *ctx->opt == 'h') if (internal_help && *ctx->opt == 'h')
goto show_usage; goto show_usage;
goto unknown; goto unknown;
case PARSE_OPT_NON_OPTION:
case PARSE_OPT_HELP:
case PARSE_OPT_COMPLETE:
BUG("parse_short_opt() cannot return these");
case PARSE_OPT_DONE:
break;
} }
if (ctx->opt) if (ctx->opt)
check_typos(arg + 1, options); check_typos(arg + 1, options);
while (ctx->opt) { while (ctx->opt) {
switch (parse_short_opt(ctx, options)) { switch (parse_short_opt(ctx, options)) {
case -1: case PARSE_OPT_ERROR:
return PARSE_OPT_ERROR; return PARSE_OPT_ERROR;
case -2: case PARSE_OPT_UNKNOWN:
if (internal_help && *ctx->opt == 'h') if (internal_help && *ctx->opt == 'h')
goto show_usage; goto show_usage;
@ -617,6 +627,12 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
ctx->argv[0] = xstrdup(ctx->opt - 1); ctx->argv[0] = xstrdup(ctx->opt - 1);
*(char *)ctx->argv[0] = '-'; *(char *)ctx->argv[0] = '-';
goto unknown; goto unknown;
case PARSE_OPT_NON_OPTION:
case PARSE_OPT_COMPLETE:
case PARSE_OPT_HELP:
BUG("parse_short_opt() cannot return these");
case PARSE_OPT_DONE:
break;
} }
} }
continue; continue;
@ -635,12 +651,17 @@ int parse_options_step(struct parse_opt_ctx_t *ctx,
if (internal_help && !strcmp(arg + 2, "help")) if (internal_help && !strcmp(arg + 2, "help"))
goto show_usage; goto show_usage;
switch (parse_long_opt(ctx, arg + 2, options)) { switch (parse_long_opt(ctx, arg + 2, options)) {
case -1: case PARSE_OPT_ERROR:
return PARSE_OPT_ERROR; return PARSE_OPT_ERROR;
case -2: case PARSE_OPT_UNKNOWN:
goto unknown; goto unknown;
case -3: case PARSE_OPT_HELP:
goto show_usage; goto show_usage;
case PARSE_OPT_NON_OPTION:
case PARSE_OPT_COMPLETE:
BUG("parse_long_opt() cannot return these");
case PARSE_OPT_DONE:
break;
} }
continue; continue;
unknown: unknown:

View File

@ -49,8 +49,8 @@ struct option;
typedef int parse_opt_cb(const struct option *, const char *arg, int unset); typedef int parse_opt_cb(const struct option *, const char *arg, int unset);
struct parse_opt_ctx_t; struct parse_opt_ctx_t;
typedef int parse_opt_ll_cb(struct parse_opt_ctx_t *ctx, typedef enum parse_opt_result parse_opt_ll_cb(struct parse_opt_ctx_t *ctx,
const struct option *opt, int unset); const struct option *opt, int unset);
/* /*
* `type`:: * `type`::
@ -222,12 +222,12 @@ const char *optname(const struct option *opt, int flags);
/*----- incremental advanced APIs -----*/ /*----- incremental advanced APIs -----*/
enum { enum parse_opt_result {
PARSE_OPT_COMPLETE = -2, PARSE_OPT_COMPLETE = -3,
PARSE_OPT_HELP = -1, PARSE_OPT_HELP = -2,
PARSE_OPT_DONE, PARSE_OPT_ERROR = -1, /* must be the same as error() */
PARSE_OPT_DONE = 0, /* fixed so that "return 0" works */
PARSE_OPT_NON_OPTION, PARSE_OPT_NON_OPTION,
PARSE_OPT_ERROR,
PARSE_OPT_UNKNOWN PARSE_OPT_UNKNOWN
}; };