OPTION_CALLBACK is much simpler/safer to use, but parse_opt_cb does not allow access to parse_opt_ctx_t, which sometimes is useful (e.g. to obtain the prefix). Extending parse_opt_cb to take parse_opt_cb could result in a lot of changes. Instead let's just allow ll_callback to be used with OPTION_CALLBACK. The user will have to be careful, not to change anything in ctx, or return wrong result code. But that's the price for ll_callback. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx> --- builtin/merge.c | 2 ++ builtin/update-index.c | 20 +++++++++++++++----- parse-options-cb.c | 4 +++- parse-options.c | 42 ++++++++++++++++++++++++++++-------------- parse-options.h | 5 +++-- 5 files changed, 51 insertions(+), 22 deletions(-) diff --git a/builtin/merge.c b/builtin/merge.c index de64d7850e..563a16f38a 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -114,11 +114,13 @@ static int option_parse_message(const struct option *opt, static enum parse_opt_result option_read_message(struct parse_opt_ctx_t *ctx, const struct option *opt, + const char *arg_not_used, int unset) { struct strbuf *buf = opt->value; const char *arg; + BUG_ON_OPT_ARG(arg_not_used); if (unset) BUG("-F cannot be negated"); diff --git a/builtin/update-index.c b/builtin/update-index.c index 21c84e5590..7abde20169 100644 --- a/builtin/update-index.c +++ b/builtin/update-index.c @@ -848,13 +848,15 @@ static int parse_new_style_cacheinfo(const char *arg, } static enum parse_opt_result cacheinfo_callback( - struct parse_opt_ctx_t *ctx, const struct option *opt, int unset) + struct parse_opt_ctx_t *ctx, const struct option *opt, + const char *arg, int unset) { struct object_id oid; unsigned int mode; const char *path; BUG_ON_OPT_NEG(unset); + BUG_ON_OPT_ARG(arg); if (!parse_new_style_cacheinfo(ctx->argv[1], &mode, &oid, &path)) { if (add_cacheinfo(mode, &oid, path, 0)) @@ -874,11 +876,13 @@ static enum parse_opt_result cacheinfo_callback( } static enum parse_opt_result stdin_cacheinfo_callback( - struct parse_opt_ctx_t *ctx, const struct option *opt, int unset) + struct parse_opt_ctx_t *ctx, const struct option *opt, + const char *arg, int unset) { int *nul_term_line = opt->value; BUG_ON_OPT_NEG(unset); + BUG_ON_OPT_ARG(arg); if (ctx->argc != 1) return error("option '%s' must be the last argument", opt->long_name); @@ -888,11 +892,13 @@ static enum parse_opt_result stdin_cacheinfo_callback( } static enum parse_opt_result stdin_callback( - struct parse_opt_ctx_t *ctx, const struct option *opt, int unset) + struct parse_opt_ctx_t *ctx, const struct option *opt, + const char *arg, int unset) { int *read_from_stdin = opt->value; BUG_ON_OPT_NEG(unset); + BUG_ON_OPT_ARG(arg); if (ctx->argc != 1) return error("option '%s' must be the last argument", opt->long_name); @@ -901,12 +907,14 @@ static enum parse_opt_result stdin_callback( } static enum parse_opt_result unresolve_callback( - struct parse_opt_ctx_t *ctx, const struct option *opt, int unset) + struct parse_opt_ctx_t *ctx, const struct option *opt, + const char *arg, int unset) { int *has_errors = opt->value; const char *prefix = startup_info->prefix; BUG_ON_OPT_NEG(unset); + BUG_ON_OPT_ARG(arg); /* consume remaining arguments. */ *has_errors = do_unresolve(ctx->argc, ctx->argv, @@ -920,12 +928,14 @@ static enum parse_opt_result unresolve_callback( } static enum parse_opt_result reupdate_callback( - struct parse_opt_ctx_t *ctx, const struct option *opt, int unset) + struct parse_opt_ctx_t *ctx, const struct option *opt, + const char *arg, int unset) { int *has_errors = opt->value; const char *prefix = startup_info->prefix; BUG_ON_OPT_NEG(unset); + BUG_ON_OPT_ARG(arg); /* consume remaining arguments. */ setup_work_tree(); diff --git a/parse-options-cb.c b/parse-options-cb.c index ec01ef722b..2733393546 100644 --- a/parse-options-cb.c +++ b/parse-options-cb.c @@ -171,8 +171,10 @@ int parse_opt_noop_cb(const struct option *opt, const char *arg, int unset) * parse_options(). */ enum parse_opt_result parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, - const struct option *opt, int unset) + const struct option *opt, + const char *arg, int unset) { + BUG_ON_OPT_ARG(arg); return PARSE_OPT_UNKNOWN; } diff --git a/parse-options.c b/parse-options.c index 372f5cede4..a0ff8ea225 100644 --- a/parse-options.c +++ b/parse-options.c @@ -95,7 +95,7 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, switch (opt->type) { case OPTION_LOWLEVEL_CALLBACK: - return opt->ll_callback(p, opt, unset); + return opt->ll_callback(p, opt, NULL, unset); case OPTION_BIT: if (unset) @@ -161,16 +161,27 @@ static enum parse_opt_result get_value(struct parse_opt_ctx_t *p, return err; case OPTION_CALLBACK: + { + const char *p_arg = NULL; + int p_unset; + if (unset) - return (*opt->callback)(opt, NULL, 1) ? (-1) : 0; - if (opt->flags & PARSE_OPT_NOARG) - return (*opt->callback)(opt, NULL, 0) ? (-1) : 0; - if (opt->flags & PARSE_OPT_OPTARG && !p->opt) - return (*opt->callback)(opt, NULL, 0) ? (-1) : 0; - if (get_arg(p, opt, flags, &arg)) + p_unset = 1; + else if (opt->flags & PARSE_OPT_NOARG) + p_unset = 0; + else if (opt->flags & PARSE_OPT_OPTARG && !p->opt) + p_unset = 0; + else if (get_arg(p, opt, flags, &arg)) return -1; - return (*opt->callback)(opt, arg, 0) ? (-1) : 0; - + else { + p_unset = 0; + p_arg = arg; + } + if (opt->callback) + return (*opt->callback)(opt, p_arg, p_unset) ? (-1) : 0; + else + return (*opt->ll_callback)(p, opt, p_arg, p_unset); + } case OPTION_INTEGER: if (unset) { *(int *)opt->value = 0; @@ -238,7 +249,10 @@ static enum parse_opt_result parse_short_opt(struct parse_opt_ctx_t *p, len++; arg = xmemdupz(p->opt, len); p->opt = p->opt[len] ? p->opt + len : NULL; - rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0; + if (numopt->callback) + rc = (*numopt->callback)(numopt, arg, 0) ? (-1) : 0; + else + rc = (*numopt->ll_callback)(p, numopt, arg, 0); free(arg); return rc; } @@ -414,10 +428,10 @@ static void parse_options_check(const struct option *opts) err |= optbug(opts, "should not accept an argument"); break; case OPTION_CALLBACK: - if (!opts->callback) - BUG("OPTION_CALLBACK needs a callback"); - if (opts->ll_callback) - BUG("OPTION_CALLBACK needs no ll_callback"); + if (!opts->callback && !opts->ll_callback) + BUG("OPTION_CALLBACK needs one callback"); + if (opts->callback && opts->ll_callback) + BUG("OPTION_CALLBACK can't have two callbacks"); break; case OPTION_LOWLEVEL_CALLBACK: if (!opts->ll_callback) diff --git a/parse-options.h b/parse-options.h index 4e49185027..ce75278804 100644 --- a/parse-options.h +++ b/parse-options.h @@ -50,7 +50,8 @@ typedef int parse_opt_cb(const struct option *, const char *arg, int unset); struct parse_opt_ctx_t; 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, + const char *arg, int unset); /* * `type`:: @@ -267,7 +268,7 @@ int parse_opt_commits(const struct option *, const char *, int); int parse_opt_tertiary(const struct option *, const char *, int); int parse_opt_string_list(const struct option *, const char *, int); int parse_opt_noop_cb(const struct option *, const char *, int); -int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, int); +int parse_opt_unknown_cb(struct parse_opt_ctx_t *ctx, const struct option *, const char *, int); int parse_opt_passthru(const struct option *, const char *, int); int parse_opt_passthru_argv(const struct option *, const char *, int); -- 2.20.0.482.g66447595a7