Add a simple option parser.

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.

Quoted 7 lines · Kristian Høgsberg

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.

if we are going in that direction (and I believe it's a good one), we
should be sure that the model fits with other commands as well. And as I said on IRC, I believe the most "horrible" (as in complex) option parser in git is the one from git-grep.

A migration of git-grep on that API should be tried first. If this
works well enough, I believe that the rest of the git commands will be migrated easily enough. (with maybe small addition to parse-option.[hc] but the hardcore things should have been met with git-grep already I think).

Quoted 14 lines · Pierre Habouzit

Quoted 7 lines · Kristian Høgsberg

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.

if we are going in that direction (and I believe it's a good one), we
should be sure that the model fits with other commands as well. And as I said on IRC, I believe the most "horrible" (as in complex) option parser in git is the one from git-grep.

Quoted 5 lines

A migration of git-grep on that API should be tried first. If this
works well enough, I believe that the rest of the git commands will be migrated easily enough. (with maybe small addition to parse-option.[hc] but the hardcore things should have been met with git-grep already I think).

I'm not sure - we can go with the current proposal and add new options types and probably the callback option type I suggested as we go. I don't want to block builtin-commit on figuring out what the perfect option parser should look like and what I sent out earlier work for commit. I think the way you handled the strbuf rewrites worked pretty well; extending and rewriting the API as you put it to use in more and more places. We can do the same thing with parse_options().

Quoted 7 lines · Kristian Høgsberg

I'm not sure - we can go with the current proposal and add new options types and probably the callback option type I suggested as we go. I don't want to block builtin-commit on figuring out what the perfect option parser should look like and what I sent out earlier work for commit. I think the way you handled the strbuf rewrites worked pretty well; extending and rewriting the API as you put it to use in more and more places. We can do the same thing with parse_options().

Of course we can do that, or junio said that some people talked about
popt some time ago. I understand that you don't want to block the git-commit work, but doing things right from the beginning is often a big win on the long term.

I don't know popt, and I don't know if it has sufficient expressivity.
For sure I don't like getopt_long APIs at all, so if popt is as cumbersome, rolling our own based on the current parse_options you propose is probably a good choice.

Okay, popt seems to be quite complicated, and depends upon gettext

(which we may require as per survey results, but right now it seems a useless dependency).

+/* Parse the given options against the list of known options. The
+ * order of the option structs matters, in that ambiguous
+ * abbreviations (eg, --in could be short for --include or
+ * --interactive) are matched by the first option that share the
+ * prefix.

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

Signed-off-by Pierre Habouzit <[email protected]>

---

I'm sorry about the "From" I don't intend to "steal" the patch in any sense, it's just an alternate proposal.

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

 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

Makefile +1 -1

index 62bdac6..d90e959 100644
@@ -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 \

parse-options.c new file +13 -0

new file mode 100644
index 0000000..eb3ff40
@@ -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;

parse-options.c new file +130 -0

+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;
+}

parse-options.h new file +29 -0

new file mode 100644
index 0000000..e4749d0
@@ -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

Quoted 15 lines · Pierre Habouzit

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".

Quoted 2 lines

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.

Quoted 2 lines

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().

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).

Quoted 8 lines · Pierre Habouzit

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.

Quoted 2 lines

Aggregation of single switches is allowed:
-rC0 is the same as -r -C 0 (supposing that -C wants an arg).

I like options aggregation, but I'm not sure aggregating option arguments is a good idea... I can't even think of an application that does it.

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Pierre Habouzit
Sent: Friday, October 05, 2007 9:46 AM
To: Mike Hommey
Cc: Kristian Høgsberg; [email protected]; Junio C Hamano
Subject: Re: [ALTERNATE PATCH] Add a simple option parser.

Quoted 1 line · Mike Hommey

On Fri, Oct 05, 2007 at 04:25:07PM +0200, Pierre Habouzit

Quoted 2 lines · <[email protected]> wrote:

Quoted 2 lines

The option parser takes argc, argv, an array of struct option and a usage string. Each of the struct option elements

in the array

Quoted 1 line

Quoted 1 line

describes a valid option, its type and a pointer to the

location where the

Quoted 1 line

Quoted 1 line

value is written. The entry point is parse_options(),

which scans through

Quoted 1 line

Quoted 1 line

the given argv, and matches each option there against the

list of valid

Quoted 2 lines

Quoted 2 lines

options. During the scan, argv is rewritten to only contain the non-option command line arguments and the number of these

is returned.

Quoted 5 lines

Quoted 3 lines

Aggregation of single switches is allowed:
-rC0 is the same as -r -C 0 (supposing that -C wants an arg).

I like options aggregation, but I'm not sure aggregating

option arguments

Quoted 1 line

is a good idea... I can't even think of an application that does it.

You mean like grep -A1 or diff -u3 or ls -w10 ?

getopt does that by default as well, so you may not have aware of it, but it's how things work in your system already.

btw ls -rw10 works, though ls -w10r drops the 'r'
silently. FWIW I
don't, in that case, the alternate patch I propose complains about "10r"
not being a valid integer, and that's because unlike getopt, the patch krh proposed knows what an integer is ;)

Signature · 4 lines

·O·  Pierre Habouzit
··O                                                [email protected]
OOO                                                
http://www.madism.org

You probably already considered and rejected the GNU argp parser. I used it before and I'd like to know reasons I should stay away from it.

-----Original Message-----
From: Pierre Habouzit [mailto:[email protected]]
Sent: Friday, October 05, 2007 10:57 AM
To: Medve Emilian-EMMEDVE1
Cc: Mike Hommey; Kristian Høgsberg; [email protected]; Junio C Hamano
Subject: Re: [ALTERNATE PATCH] Add a simple option parser.

Quoted 3 lines · On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:

You probably already considered and rejected the GNU argp parser. I used it before and I'd like to know reasons I should stay away from it.

Because it's GNU and that it's a heavy dependency to begin with.

Moreover, getopt_long doesn't deal with argument types (like integers).

Hi Pierre,

Quoted 8 lines

-----Original Message-----
From: Pierre Habouzit [mailto:[email protected]]
Sent: Friday, October 05, 2007 10:57 AM
To: Medve Emilian-EMMEDVE1
Cc: Mike Hommey; Kristian Høgsberg; [email protected]; Junio C Hamano
Subject: Re: [ALTERNATE PATCH] Add a simple option parser.

Quoted 6 lines · On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:

Quoted 3 lines

You probably already considered and rejected the GNU argp parser. I used it before and I'd like to know reasons I should stay away from it.

Because it's GNU and that it's a heavy dependency to begin with.

So it's more of a political decision then a technical one?

Quoted 15 lines · "Medve Emilian-EMMEDVE1" <[email protected]> writes:

Hi Pierre,

Quoted 13 lines

-----Original Message-----
From: Pierre Habouzit [mailto:[email protected]]
Sent: Friday, October 05, 2007 10:57 AM
To: Medve Emilian-EMMEDVE1
Cc: Mike Hommey; Kristian Høgsberg; [email protected]; Junio C Hamano
Subject: Re: [ALTERNATE PATCH] Add a simple option parser.

Quoted 3 lines · On ven, oct 05, 2007 at 03:45:36 +0000, Medve Emilian-EMMEDVE1 wrote:

You probably already considered and rejected the GNU argp parser. I used it before and I'd like to know reasons I should stay away from it.

Quoted 3 lines

Quoted 1 line

Because it's GNU and that it's a heavy dependency to begin with.

So it's more of a political decision then a technical one?

Well, if it is GNU then it is likely to mean GPLv3 (or GPLv3+) at some point of time, though it should certainly be possible for now to still secure a v2-licensed version (either GPL or LGPL).

And the typical git developer AFAICT prefers to consider themselves as unaligned with GNU and the FSF as much as possible.

The real issue is dependency and bloat. getopt_long would need the GNU
implementation, That I believe depends upon gettext, and argp is just bloated, and I'm not even sure it's distributed outside from the glibc anyways.

Quoted 2 lines

Because it's GNU and that it's a heavy dependency to begin with.

So it's more of a political decision then a technical one?

Quoted 4 lines

Quoted 2 lines

Because it's GNU and that it's a heavy dependency to begin with.

So it's more of a political decision then a technical one?

I'd *strongly* argue against new dependencies unless they buy us something major.

We've been good at cutting them down, including any required libraries
internally. We shouldn't add new ones.

So we'd have to include GNU getopt sources with the git tree, at which
point any advantage would be gone. Might as well include a private and
simpler version of our own.

From what I understand argp is part of glibc.

Hello Linus,

Quoted 12 lines · On Fri, 5 Oct 2007, Medve Emilian-EMMEDVE1 wrote:

Quoted 4 lines

Quoted 2 lines

Because it's GNU and that it's a heavy dependency to begin with.

So it's more of a political decision then a technical one?

I'd *strongly* argue against new dependencies unless they buy us something major.

We've been good at cutting them down, including any required libraries internally. We shouldn't add new ones.

Quoted 3 lines

So we'd have to include GNU getopt sources with the git tree, at which point any advantage would be gone. Might as well include a private and simpler version of our own.

From what I understand argp is part of glibc.

From what I understand argp is part of glibc.

Quoted 8 lines · Pierre Habouzit

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.

Quoted 2 lines

Aggregation of single switches is allowed:
-rC0 is the same as -r -C 0 (supposing that -C wants an arg).

I like options aggregation, but I'm not sure aggregating option arguments is a good idea... I can't even think of an application that does it.