(+cc: Stephen)
Dmitry Ivankov wrote:
> 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.]
>
> Signed-off-by: Dmitry Ivankov <divanorama@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>
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 {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html