Re: [PATCH v3 1/2] parse-options: export opterr, optbug
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Aug 11, 2011, 10:42 UTC
- Message-ID
- <20110811104236.GB5012@elie.gateway.2wire.net>
- In-Reply-To
- <1313054138-30885-2-git-send-email-divanorama@gmail.com>
(+cc: Stephen) Dmitry Ivankov wrote:
Show 5 quoted lines
> opterror and optbug functions are used by some of parsing routines > in parse-options.c to report errors and bugs respectively. > > Export these functions to allow more custom parsing routines to use > them in a uniform way.
In other words, exposing opterror() allows custom option types to behave more like the built-in ones when producing messages like "option `<opt>' expects a numerical value". What should they pass in the "flags" argument? Does this deserve a mention in the "Option Callbacks" section of Documentation/technical/api-parse-options.txt?
Would opterror() be enough? I don't see any current users of optbug outside of parse_options_check() (which is part of low-level machinery).
Aside from that, seems sensible.
[quoting in full for Stephen's convenience. One quick comment below.]
Show 39 quoted lines
>
> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com>
> ---
> parse-options.c | 4 ++--
> parse-options.h | 2 ++
> 2 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/parse-options.c b/parse-options.c
> index 879ea82..7b061af 100644
> --- a/parse-options.c
> +++ b/parse-options.c
> @@ -12,14 +12,14 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx,
> #define OPT_SHORT 1
> #define OPT_UNSET 2
>
> -static int optbug(const struct option *opt, const char *reason)
> +int optbug(const struct option *opt, const char *reason)
> {
> if (opt->long_name)
> return error("BUG: option '%s' %s", opt->long_name, reason);
> return error("BUG: switch '%c' %s", opt->short_name, reason);
> }
>
> -static int opterror(const struct option *opt, const char *reason, int flags)
> +int opterror(const struct option *opt, const char *reason, int flags)
> {
> if (flags & OPT_SHORT)
> return error("switch `%c' %s", opt->short_name, reason);
> diff --git a/parse-options.h b/parse-options.h
> index 05eb09b..59e0b52 100644
> --- a/parse-options.h
> +++ b/parse-options.h
> @@ -165,6 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg,
> const char * const *usagestr,
> const struct option *options);
>
> +extern int optbug(const struct option *opt, const char *reason);
> +extern int opterror(const struct option *opt, const char *reason, int flags);
> /*----- incremental advanced APIs -----*/A blank line above the comment would make this more readable.
>
> enum {
> -- Except as noted above, Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>
Some (though not all) of the possible changes noted above implemented below, plus an example caller (which made reading the patch a little easier). What do you think?
builtin/read-tree.c | 2 +- parse-options.c | 2 +- parse-options.h | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-)
diff --git c/builtin/read-tree.c i/builtin/read-tree.c index df6c4c88..6013090c 100644 --- c/builtin/read-tree.c +++ i/builtin/read-tree.c @@ -53,7 +53,7 @@ static int exclude_per_directory_cb(const struct option *opt, const char *arg, opts = (struct unpack_trees_options *)opt->value; if (opts->dir) - die("more than one --exclude-per-directory given."); + return opterror(opt, "can only be supplied once", 0); dir = xcalloc(1, sizeof(*opts->dir)); dir->flags |= DIR_SHOW_IGNORED; diff --git c/parse-options.c i/parse-options.c index 7b061afc..777611b1 100644 --- c/parse-options.c +++ i/parse-options.c @@ -12,7 +12,7 @@ static int parse_options_usage(struct parse_opt_ctx_t *ctx, #define OPT_SHORT 1 #define OPT_UNSET 2 -int optbug(const struct option *opt, const char *reason) +static int optbug(const struct option *opt, const char *reason) { if (opt->long_name) return error("BUG: option '%s' %s", opt->long_name, reason); diff --git c/parse-options.h i/parse-options.h index 59e0b524..6d31ad3a 100644 --- c/parse-options.h +++ i/parse-options.h @@ -165,8 +165,8 @@ extern NORETURN void usage_msg_opt(const char *msg, const char * const *usagestr, const struct option *options); -extern int optbug(const struct option *opt, const char *reason); extern int opterror(const struct option *opt, const char *reason, int flags); + /*----- incremental advanced APIs -----*/ enum {