Re: [ALTERNATE PATCH] Add a simple option parser.

["Kristian Høgsberg" <krh@redhat.com>]

On Fri, 2007-10-05 at 16:25 +0200, Pierre Habouzit wrote:
> The option parser takes argc, argv, an array of struct option
> and a usage string.  Each of the struct option elements in the array
> describes a valid option, its type and a pointer to the location where the
> value is written.  The entry point is parse_options(), which scans through
> the given argv, and matches each option there against the list of valid
> options.  During the scan, argv is rewritten to only contain the
> non-option command line arguments and the number of these is returned.
> 
> Aggregation of single switches is allowed:
>   -rC0 is the same as -r -C 0 (supposing that -C wants an arg).
> 
> Boolean switches automatically support the option with the same name,
> prefixed with 'no-' to disable the switch:
>   --no-color / --color only need to have an entry for "color".
> 
> Long options are supported either with '=' or without:
>   --some-option=foo is the same as --some-option foo

That looks great, works for me.  One comment, though: it looks like
you're not sure whether to call these things "options" or "switches".
We should choose one and stick with it.

Acked-by: Kristian Høgsberg <krh@redhat.com>

> Signed-off-by: Pierre Habouzit <madcoder@debian.org>
> ---
> 
> I'm sorry about the "From" I don't intend to "steal" the patch in any
> sense, it's just an alternate proposal.

No worries, I'm glad to see this move forward.

> oh and I don't grok what OPTION_LAST is for, so I left it apart, but
> it seems unused ?

Oh, kill that.  I used that as the option array terminator before we
switched to ARRAY_SIZE().

> 
>  Makefile        |    2 +-
>  parse-options.c |  154 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  parse-options.h |   29 ++++++++++
>  3 files changed, 184 insertions(+), 1 deletions(-)
>  create mode 100644 parse-options.c
>  create mode 100644 parse-options.h
> 
> diff --git a/Makefile b/Makefile
> index 62bdac6..d90e959 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -310,7 +310,7 @@ LIB_OBJS = \
>  	alloc.o merge-file.o path-list.o help.o unpack-trees.o $(DIFF_OBJS) \
>  	color.o wt-status.o archive-zip.o archive-tar.o shallow.o utf8.o \
>  	convert.o attr.o decorate.o progress.o mailmap.o symlinks.o remote.o \
> -	transport.o bundle.o
> +	transport.o bundle.o parse-options.o
>  
>  BUILTIN_OBJS = \
>  	builtin-add.o \
> diff --git a/parse-options.c b/parse-options.c
> new file mode 100644
> index 0000000..eb3ff40
> --- /dev/null
> +++ b/parse-options.c
> @@ -0,0 +1,154 @@
> +#include "git-compat-util.h"
> +#include "parse-options.h"
> +
> +struct optparse_t {
> +	const char **argv;
> +	int argc;
> +	const char *opt;
> +};
> +
> +static inline const char *skippfx(const char *str, const char *prefix)
> +{
> +	size_t len = strlen(prefix);
> +	return strncmp(str, prefix, len) ? NULL : str + len;
> +}
> +
> +static int opterror(struct option *opt, const char *reason, int shorterr)
> +{
> +	if (shorterr) {
> +		return error("switch `%c' %s", opt->short_name, reason);
> +	} else {
> +		return error("option `%s' %s", opt->long_name, reason);
> +	}
> +}

option/switch?

> +static int get_value(struct optparse_t *p, struct option *opt,
> +					 int boolean, int shorterr)
> +{
> +	switch (opt->type) {
> +		const char *s;
> +		int v;
> +
> +	  case OPTION_BOOLEAN:
> +		*(int *)opt->value = boolean;
> +		return 0;
> +
> +	  case OPTION_STRING:
> +		if (p->opt && *p->opt) {
> +			*(const char **)opt->value = p->opt;
> +			p->opt = NULL;
> +		} else {
> +			if (p->argc < 1)
> +				return opterror(opt, "requires a value", shorterr);
> +			*(const char **)opt->value = *++p->argv;
> +			p->argc--;
> +		}
> +		return 0;
> +
> +	  case OPTION_INTEGER:
> +		if (p->opt && *p->opt) {
> +			v = strtol(p->opt, (char **)&s, 10);
> +			p->opt = NULL;
> +		} else {
> +			if (p->argc < 1)
> +				return opterror(opt, "requires a value", shorterr);
> +			v = strtol(*++p->argv, (char **)&s, 10);
> +			p->argc--;
> +		}
> +		if (*s)
> +			return opterror(opt, "expects a numerical value", shorterr);
> +		*(int *)opt->value = v;
> +		return 0;
> +	}
> +
> +	abort();
> +}
> +
> +static int parse_short_opt(struct optparse_t *p, struct option *options, int count)
> +{
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		if (options[i].short_name == *p->opt) {
> +			p->opt++;
> +			return get_value(p, options + i, 1, 1);
> +		}
> +	}
> +	return error("unknown switch `%c'", *p->opt);
> +}
> +
> +static int parse_long_opt(struct optparse_t *p, const char *arg,
> +                          struct option *options, int count)
> +{
> +	int boolean = 1;
> +	int i;
> +
> +	for (i = 0; i < count; i++) {
> +		const char *rest;
> +		
> +		if (!options[i].long_name)
> +			continue;
> +
> +		rest = skippfx(arg, options[i].long_name);
> +		if (!rest && options[i].type == OPTION_BOOLEAN) {
> +			if (!rest && skippfx(arg, "no-")) {
> +				rest = skippfx(arg + 3, options[i].long_name);
> +				boolean = 0;
> +			}
> +			if (rest && *rest == '=')
> +				return opterror(options + i, "takes no value", 0);
> +		}
> +		if (!rest || (*rest && *rest != '='))
> +			continue;
> +		if (*rest) {
> +			p->opt = rest;
> +		}
> +		return get_value(p, options + i, boolean, 0);
> +	}
> +	return error("unknown option `%s'", arg);
> +}
> +
> +int parse_options(int argc, const char **argv,
> +		  struct option *options, int count,
> +		  const char *usage_string)
> +{
> +	struct optparse_t optp = { argv + 1, argc - 1, NULL };
> +	int j = 0;
> +
> +	while (optp.argc) {
> +		const char *arg = optp.argv[0];
> +
> +		if (*arg != '-' || !arg[1]) {
> +			argv[j++] = *optp.argv++;
> +			optp.argc--;
> +			continue;
> +		}
> +
> +		if (arg[1] != '-') {
> +			optp.opt = arg + 1;
> +			while (*optp.opt) {
> +				if (parse_short_opt(&optp, options, count) < 0) {
> +					usage(usage_string);
> +					return -1;
> +				}
> +			}
> +			optp.argc--;
> +			optp.argv++;
> +			continue;
> +		}
> +
> +		if (!arg[2]) /* "--" */
> +			break;
> +
> +		if (parse_long_opt(&optp, arg + 2, options, count)) {
> +			usage(usage_string);
> +			return -1;
> +		}
> +		optp.argc--;
> +		optp.argv++;
> +	}
> +
> +	memmove(argv + j, optp.argv, optp.argc * sizeof(argv));
> +	argv[j + optp.argc] = NULL;
> +	return j + optp.argc;
> +}
> diff --git a/parse-options.h b/parse-options.h
> new file mode 100644
> index 0000000..e4749d0
> --- /dev/null
> +++ b/parse-options.h
> @@ -0,0 +1,29 @@
> +#ifndef PARSE_OPTIONS_H
> +#define PARSE_OPTIONS_H
> +
> +enum option_type {
> +	OPTION_BOOLEAN,
> +	OPTION_STRING,
> +	OPTION_INTEGER,
> +#if 0
> +	OPTION_LAST,
> +#endif
> +};
> +
> +struct option {
> +	enum option_type type;
> +	const char *long_name;
> +	char short_name;
> +	void *value;
> +};
> +
> +/* parse_options() will filter out the processed options and leave the
> + * non-option argments in argv[].  The return value is the number of
> + * arguments left in argv[].
> + */
> +
> +extern int parse_options(int argc, const char **argv,
> +			 struct option *options, int count,
> +			 const char *usage_string);
> +
> +#endif