threads / rfc / 4135

RFC patchgitopt - command-line parsing enhancements (take #2)

Subject: [PATCH/RFC] gitopt - command-line parsing enhancements (take #2)

## tl;dr

12 messages between May 14, 2006 and May 16, 2006. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Eric Wong· May 14, 2006, 15:19 UTC · lore
This should be a less scary series of patches for gitopt.
[PATCH 1/5] gitopt: a new command-line option parser for git
[PATCH 2/5] gitopt: convert ls-files, ls-tree, update-index
	Simple conversions.
[PATCH 3/5] gitopt: convert setup_revisions() and friends
	This one is pretty big, some extra testing + review would be
	nice.
[PATCH 4/5] commit: allow --pretty= args to be abbreviated
	Not strictly related to gitopt, but finger-friendly nevertheless.
[PATCH 5/5] diff: parse U/u/unified options with an optional integer arg
	Originally, this was bundled into:
		<11471512123005-git-send-email-normalperson@yhbt.net>,
	Then Linus did a more correct one that didn't forget combine-diff:
		<Pine.LNX.4.64.0605131317200.3866@g5.osdl.org>
	This one combines both of them.
Eric Wong· May 14, 2006, 15:19 UTC · re: Eric Wong · lore

[PATCH 1/5] gitopt: a new command-line option parser for git

It was initially conceived as an addition to the git.c wrapper, and not affect other programs. But it turns out existing C programs can use it pretty easily, too.

Features include:
 * getopt-style permuting (can easily be disabled for
   things like update-index)
 * command-line compatibile with existing usage:
   -S=pickaxe-arg-with-leading-equals is unchanged
 * printf-style rewriting (for front-ending shell scripts)
 * unbundling of short options: -un20z => -u -n20 -z
 * automatically understands unambiguous abbreviations
 * optional argument handling (-C<num>, -M<num>)
   -C <num> (with a space between them) has not changed,
   however, <num> can be a sha1, or a path
Changes from the initial patch:
 * automatically handle rewrites when not in pass-through mode (pass-through
   mode is used for git wrapper only, usually for shell scripts).
 * Fixed an off-by-one error in parse_bundled that could cause a segfault
 * Supports being called as an iterator mode, as suggested by
   Junio, meaning:
 * no additional global variables required for converting
   existing C programs
 * no more scary macros :)
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 .gitignore           |    1 
 Makefile             |   10 +
 git.c                |   11 +
 gitopt.c             |  662 ++++++++++++++++++++++++++++++++++++++++++++++++++
 gitopt.h             |  120 +++++++++
 gitopt/git_wrapper.h |   45 +++
 gitopt/sh.h          |   45 +++
 t/t0200-gitopt.sh    |  295 ++++++++++++++++++++++
 test-gitopt.c        |  112 ++++++++
 9 files changed, 1296 insertions(+), 5 deletions(-)
 create mode 100644 gitopt.c
 create mode 100644 gitopt.h
 create mode 100644 gitopt/git_wrapper.h
 create mode 100644 gitopt/sh.h
 create mode 100755 t/t0200-gitopt.sh
 create mode 100644 test-gitopt.c
eea531f2f17355161d58b61c3371f496f12c5364
Show changes to 9 files +1296 −5

.gitignore, Makefile, git.c, gitopt.c, gitopt.h, gitopt/git_wrapper.h, gitopt/sh.h, t/t0200-gitopt.sh, test-gitopt.c

diff --git a/.gitignore b/.gitignore
index b5959d6..b2d8b06 100644
--- a/.gitignore
+++ b/.gitignore
@@ -123,6 +123,7 @@ git-write-tree
 git-core-*/?*
 test-date
 test-delta
+test-gitopt
 common-cmds.h
 *.tar.gz
 *.dsc
diff --git a/Makefile b/Makefile
index 37fbe78..8fd3e13 100644
--- a/Makefile
+++ b/Makefile
@@ -197,7 +197,7 @@ LIB_H = \
 	blob.h cache.h commit.h csum-file.h delta.h \
 	diff.h object.h pack.h pkt-line.h quote.h refs.h \
 	run-command.h strbuf.h tag.h tree.h git-compat-util.h revision.h \
-	tree-walk.h log-tree.h
+	tree-walk.h log-tree.h gitopt.h
 
 DIFF_OBJS = \
 	diff.o diff-lib.o diffcore-break.o diffcore-order.o \
@@ -212,6 +212,7 @@ LIB_OBJS = \
 	server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
 	tag.o tree.o usage.o config.o environment.o ctype.o copy.o \
 	fetch-clone.o revision.o pager.o tree-walk.o xdiff-interface.o \
+	gitopt.o \
 	$(DIFF_OBJS)
 
 BUILTIN_OBJS = \
@@ -470,6 +471,8 @@ all:
 strip: $(PROGRAMS) git$X
 	$(STRIP) $(STRIP_OPTS) $(PROGRAMS) git$X
 
+gitopt.o: gitopt.c gitopt.h gitopt/*.h
+
 git$X: git.c common-cmds.h $(BUILTIN_OBJS) $(GITLIBS)
 	$(CC) -DGIT_VERSION='"$(GIT_VERSION)"' \
 		$(ALL_CFLAGS) -o $@ $(filter %.c,$^) \
@@ -600,7 +603,7 @@ # with that.
 
 export NO_PYTHON
 
-test: all
+test: all test-gitopt$X
 	$(MAKE) -C t/ all
 
 test-date$X: test-date.c date.o ctype.o
@@ -609,6 +612,9 @@ test-date$X: test-date.c date.o ctype.o
 test-delta$X: test-delta.c diff-delta.o patch-delta.o
 	$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $^
 
+test-gitopt$X: test-gitopt.c gitopt.o ctype.o usage.o
+	$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) $^
+
 check:
 	for i in *.c; do sparse $(ALL_CFLAGS) $(SPARSE_FLAGS) $$i || exit; done
 
diff --git a/git.c b/git.c
index 49ba518..785d97f 100644
--- a/git.c
+++ b/git.c
@@ -11,6 +11,7 @@ #include <stdarg.h>
 #include "git-compat-util.h"
 #include "exec_cmd.h"
 
+#include "gitopt/git_wrapper.h"
 #include "builtin.h"
 
 static void prepend_to_path(const char *dir, int len)
@@ -72,6 +73,7 @@ int main(int argc, const char **argv, ch
 	char *slash = strrchr(cmd, '/');
 	char git_command[PATH_MAX + 1];
 	const char *exec_path = NULL;
+	struct exec_args *ea;
 
 	/*
 	 * Take the basename of argv[0] as the command
@@ -99,7 +101,8 @@ int main(int argc, const char **argv, ch
 	if (!strncmp(cmd, "git-", 4)) {
 		cmd += 4;
 		argv[0] = cmd;
-		handle_internal_command(argc, argv, envp);
+		ea = gitopt_parse_git(argc, argv);
+		handle_internal_command(ea->argc, ea->argv, envp);
 		die("cannot handle %s internally", cmd);
 	}
 
@@ -144,6 +147,8 @@ int main(int argc, const char **argv, ch
 	}
 	argv[0] = cmd;
 
+	ea = gitopt_parse_git(argc, argv);
+
 	/*
 	 * We search for git commands in the following order:
 	 *  - git_exec_path()
@@ -157,10 +162,10 @@ int main(int argc, const char **argv, ch
 	prepend_to_path(exec_path, strlen(exec_path));
 
 	/* See if it's an internal command */
-	handle_internal_command(argc, argv, envp);
+	handle_internal_command(ea->argc, ea->argv, envp);
 
 	/* .. then try the external ones */
-	execv_git_cmd(argv);
+	execv_git_cmd(ea->argv);
 
 	if (errno == ENOENT)
 		cmd_usage(0, exec_path, "'%s' is not a git-command", cmd);
diff --git a/gitopt.c b/gitopt.c
new file mode 100644
index 0000000..9e85247
--- /dev/null
+++ b/gitopt.c
@@ -0,0 +1,662 @@
+#include "git-compat-util.h"
+#include "gitopt.h"
+#include "cache.h"
+
+/* whether or not to pass-through unrecognized switches or to report an error.
+ * This is only intended to be set to true for use in the git.c wrapper */
+int gitopt_pass_through = 0;
+static int gitopt_errno = 0;
+int gitopt_dd_seen = 0;	/* double-dash seen flag */
+
+struct exec_args *new_exec_args(const int argc)
+{
+	struct exec_args *ea = xmalloc(sizeof(*ea));
+	ea->argc = argc;
+	ea->argv = xcalloc((argc+1), sizeof(char*));
+
+	return ea;
+}
+
+struct exec_args *nil_exec_args(struct exec_args *ea)
+{
+	int i;
+	for (i = 0; i < ea->argc; i++)
+		ea->argv[i] = NULL;
+	ea->argc = 0;
+	return ea;
+}
+
+static int combine_exec_args(struct exec_args *dest, struct exec_args *from)
+{
+	int i;
+	size_t nr = 4 + dest->argc + from->argc;
+
+	dest->argv = xrealloc(dest->argv, nr * sizeof(char*));
+
+	for (i = 0; i < from->argc; i++)
+		dest->argv[dest->argc++] = from->argv[i];
+
+	dest->argv[dest->argc] = NULL;
+
+	return i;
+}
+
+static struct exec_args *rewrite_args(const char *rewrite_fmt, const char *arg)
+{
+	struct exec_args *ea;
+	size_t len = strlen(rewrite_fmt) + (arg ? strlen(arg) : 0);
+	char *dest = xmalloc(len); /* don't free this */
+	int nr_ws = 0;
+	char *a, *b;
+
+	if (!arg) {
+		strcpy(dest, rewrite_fmt);
+		if ((a = strstr(dest,"=%s")) || (a = strstr(dest," %s")))
+			a[0] = a[1] = a[2] = '\0';
+		else if ((a = strstr(dest,"%s")))
+			a[0] = a[1] = '\0';
+	} else {
+		const char *c = rewrite_fmt;
+
+		do { if (isspace(*c++)) nr_ws++; } while (*c);
+		snprintf(dest, len, rewrite_fmt, arg);
+	}
+
+	ea = new_exec_args(1 + nr_ws);
+	a = b = dest;
+
+	for (ea->argc = 0; nr_ws && *b; b++) {
+		if (isspace(*b)) {
+			*b = '\0';
+			if (strlen(a))
+				ea->argv[ea->argc++] = a;
+			a = b + 1;
+		}
+	}
+	if (strlen(a))
+		ea->argv[ea->argc++] = a;
+	ea->argv[ea->argc] = NULL;
+
+	return ea;
+}
+
+static struct exec_args *one_arg(const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	const char *c = argv[*argc_pos];
+	struct exec_args *ea = NULL;
+	const char *a = NULL;
+	size_t l_len = s->l ? strlen(s->l) : 0;
+	int so = 0; /* short option passed flag */
+
+	if (*c == '-')  {
+		if (s->s && c[1] != '-') {
+			if (isspace(s->s)) {
+				a = c + 1;
+				so = 1;
+			} else if (c[1] == s->s) {
+				a = c + 2;
+				so = 1;
+			}
+		} else if (s->l && c[1] == '-' && !strncmp(c+2, s->l, l_len))
+			a = c + 2 + l_len;
+	}
+
+	if (!a)
+		return NULL;
+
+	switch(a[0]) {
+	case '\0':
+		if (((*argc_pos + 1) < argc) && (a = argv[*argc_pos + 1])) {
+			/* optional arguments must be attached to the switch
+			 * so that there are no abiguities */
+			if (s->arg_fl & ARG_IS_OPT)
+				break;
+			*argc_pos += 1;
+			if (s->rewrite_fmt)
+				ea = rewrite_args(s->rewrite_fmt, a);
+			else if (gitopt_pass_through) {
+				ea = new_exec_args(2);
+				ea->argv[0] = c;
+				ea->argv[1] = a;
+			} else if (s->arg_fl & ARG_ONE)
+				ea = rewrite_args("%s", a);
+		}
+		break;
+	default:
+		if (!so) {
+			/* only long options get to use "=" to denote an arg */
+			/* Make sure -S'= assigned_val;' still works */
+			if (*a == '=')
+				a++;
+			else
+				return NULL;
+		}
+		if (s->rewrite_fmt)
+			ea = rewrite_args(s->rewrite_fmt, a);
+		else if (gitopt_pass_through) {
+			ea = new_exec_args(1);
+			ea->argv[0] = c;
+		} else {
+			if (s->arg_fl & ARG_IS_OPT) {
+				ea = new_exec_args(2);
+				ea->argv[0] = "ignore";
+				ea->argv[1] = a;
+			} else if (s->arg_fl & ARG_ONE) {
+				ea = new_exec_args(1);
+				ea->argv[0] = a;
+			}
+		}
+	}
+
+	return ea;
+}
+
+static struct exec_args *optional_arg_common(struct exec_args *ea,
+			const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	if (!ea) {
+		if (s->rewrite_fmt)
+			ea = rewrite_args(s->rewrite_fmt, NULL);
+		else {
+			ea = new_exec_args(1);
+			ea->argv[0] = argv[*argc_pos];
+		}
+	}
+	return ea;
+}
+
+struct exec_args *optional_arg(const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	return optional_arg_common( one_arg(s, argc, argv, argc_pos),
+						s, argc, argv, argc_pos);
+}
+
+static struct exec_args *int_arg(const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	struct exec_args *ea = one_arg(s, argc, argv, argc_pos);
+
+	if (ea) {
+		const char *c = ea->argv[ea->argc - 1];
+		char *endptr;
+		long int tmp;
+
+		/* -C<num>: */
+		if (ea->argc == 1) {
+			while (*c && !isdigit(*c)) c++;
+			if (!c) goto err;
+		}
+
+		endptr = (char *)c;
+		tmp = strtol(c, &endptr, 10);
+		if (endptr == c) {
+err:
+			if (s->arg_fl & ARG_IS_INT)
+				(*argc_pos)--;
+			free_exec_args(ea);
+			return NULL;
+		}
+	}
+	return ea;
+}
+
+static struct exec_args *optional_int_arg(const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	return optional_arg_common( int_arg(s, argc, argv, argc_pos),
+						s, argc, argv, argc_pos);
+}
+
+static struct exec_args *nr_args(int nr, const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	struct exec_args *ea;
+
+	nr++;
+	ea = new_exec_args(nr);
+	ea->argc = 1;
+	ea->argv[0] = argv[*argc_pos];
+	while (*argc_pos < (argc-1) && ea->argc < nr)
+		ea->argv[ea->argc++] = argv[++(*argc_pos)];
+	if (ea->argc != nr)
+		gitopt_errno = error("%s needed %d arguments, to %d",
+				ea->argv[0], nr-1, ea->argc - 1);
+	return ea;
+}
+
+static struct exec_args *run_proc(const struct opt_spec *s,
+			const int argc, const char **argv, int *argc_pos)
+{
+	switch (s->arg_fl) {
+	case ARG_ONE:
+		return one_arg(s, argc, argv, argc_pos);
+	case ARG_INT:
+		return int_arg(s, argc, argv, argc_pos);
+	case ARG_OPT:
+		return optional_arg(s, argc, argv, argc_pos);
+	case ARG_OPTINT:
+		return optional_int_arg(s, argc, argv, argc_pos);
+	case ARG_THREE:
+		return nr_args(3, s, argc, argv, argc_pos);
+	case ARG_TWO:
+		return nr_args(2, s, argc, argv, argc_pos);
+	default:
+		return NULL;
+	}
+}
+
+static const char * parse_bundled(struct gitopt_iterator *gi,
+			const struct opt_spec *s, const char *cur)
+{
+	struct exec_args *ea = NULL;
+	const char *orig = cur;
+	char *c = xmalloc(strlen(cur) + 2); /* don't free this */
+	const char *tmp_argv[] = { c };
+	int i = 0;
+
+	*c++ = '-';
+	*c++ = *cur++;
+	if (!s || !s->arg_fl) {
+		ea = new_exec_args(1);
+		if (s && s->rewrite_fmt)
+			ea->argv[0] = s->rewrite_fmt;
+		else {
+			ea->argv[0] = tmp_argv[0];
+			*c = '\0';
+		}
+	} else if (s->arg_fl) {
+		if (*cur) {
+			/* no space between the arg and opt switch: */
+			if (s->arg_fl & ARG_IS_INT) {
+				/* we know to handle stuff like:
+				 * -h24w80 => -h=24 -w=80 */
+				char *endptr = (char *)cur;
+				strtol(cur, &endptr, 10);
+
+				while (cur < endptr)
+					*c++ = *cur++;
+			} else if (s->arg_fl & ARG_ONE) {
+				/* unfortunately, other args are less
+				 * clear-cut */
+				while (*cur)
+					*c++ = *cur++;
+			}
+			*c = '\0';
+			ea = run_proc(s, 1, tmp_argv, &i);
+		} else if ((gi->pos + 1) < gi->argc) {
+			int j = gi->pos;
+			int x = gi->argc - j + 1;
+			const char **argv2 = xmalloc(x * sizeof(char *));
+
+			*c = '\0';
+			argv2[i++] = tmp_argv[0];
+			while (i < x) argv2[i++] = gi->argv[++j];
+
+			i = 0;
+			ea = run_proc(s, x, argv2, &i);
+			gi->pos += i;
+			free(argv2);
+		}
+	}
+	if (ea) {
+		combine_exec_args(gi->ea, ea);
+		free_exec_args(ea);
+	} else
+		gitopt_errno = error("Failed to parse bundled arguments in: %s",
+									orig);
+	return cur;
+}
+
+static int unbundle_iter(struct gitopt_iterator *gi, const struct opt_spec *ost)
+{
+	const struct opt_spec *s;
+	const char *c, *cur = gi->argv[gi->pos];
+
+	if (!gi->upos || gi->upos < cur || gi->upos > strrchr(cur,0))
+		gi->upos = cur + 1; /* only short options */
+	c = gi->upos;
+
+	while (*c) {
+		int i;
+		for (i = 0; ost[i].s || ost[i].l; i++) {
+			s = ost + i;
+			if (!s->s || isspace(s->s) || s->s != *c)
+				continue;
+			c = parse_bundled(gi, s, c);
+			if (c != gi->upos) {
+				gi->upos = c;
+				return s->id;
+			}
+			break;
+		}
+		if (ost[i].l || ost[i].s) continue;
+		/* pass-through while unbundling and creating switches:
+		 * this means that if we see -abc here, but we only
+		 * had -a defined in ost (-a defined to not accept args),
+		 * then we'd create switches
+		 * for -b and -c here (since we already knew -a)
+		 * and we're assuming -b and -c were just forgotten
+		 * in the ost.  If we had gotten -bac, that would
+		 * be passed through as -bac in gitopt_parse_ost()
+		 * as an unknown option if -b is undefined in the ost
+		 */
+		if (gitopt_pass_through) {
+			c = parse_bundled(gi, NULL, c);
+			if (c != gi->upos) {
+				gi->upos = c;
+				return GITOPT_ERROR;
+			}
+		} else {
+			/* non-fatal error, should it be non-fatal? */
+			gitopt_errno = error("Unknown option '%s' in '%s'",
+							c, gi->argv[gi->pos]);
+			c++;
+		}
+	}
+
+	gi->upos = c;
+	return GITOPT_ERROR;
+}
+
+
+static int push_one_opt(struct gitopt_iterator *gi, const struct opt_spec *s)
+{
+	struct exec_args *ea;
+
+	if (!s->arg_fl) {
+		gi->ea->argv[gi->ea->argc++] = s->rewrite_fmt ? s->rewrite_fmt
+							: gi->argv[gi->pos];
+		return s->id;
+	}
+
+	if ((ea = run_proc(s, gi->argc, gi->argv, &(gi->pos)))) {
+		combine_exec_args(gi->ea, ea);
+		free_exec_args(ea);
+		return s->id;
+	}
+	gitopt_errno = error("Failed to parse arguments for: %s %s",
+				gi->argv[gi->pos],
+				gi->argv[gi->pos+1] ? gi->argv[gi->pos+1] : "");
+	return GITOPT_ERROR;
+}
+
+/* look for a prefix abbreviation */
+static int opt_abbrev_match(const struct opt_spec *s, const char *p)
+{
+	const char *l = s->l;
+
+	while (*p) {
+		if (*l++ != *p++) return 0;
+		if (!*p || (s->arg_fl && *p == '=')) return 1;
+	}
+
+	return 0;
+}
+
+/* match a short option switch */
+static int opt_char_match(const struct opt_spec *s, const char *p)
+{
+	return ((s->s == p[0]) && ((!s->arg_fl && p[1] == '\0')
+				||
+			(s->arg_fl && (p[1] == '\0' || p[1] == '='))));
+}
+
+/* tokenize on '-' and look for a prefix abbreviation match */
+static int opt_token_match(const struct opt_spec *s, const char *p0)
+{
+	const char *l = s->l;
+	const char *p;
+
+	while ((l = strchr(l,'-'))) {
+		l++;
+		p = p0;
+		while (*p) {
+			if (*l++ != *p++) break;
+			if (!*p || (s->arg_fl && *p == '=')) return 1;
+		}
+	}
+
+	return 0;
+}
+
+/* look for unambigious abbreviated switches, if it can't be found,
+ * assume it's a non-option and pass it to b */
+static int fallback_long(struct gitopt_iterator *gi,
+			const struct opt_spec *ost, const char *cur)
+{
+	const struct opt_spec *s, *found = NULL;
+	int i;
+
+	/* look for abbreviations: */
+	for (i = 0; ost[i].l || ost[i].s; i++) {
+		s = &(ost[i]);
+		/* maybe they wanted to use a short option
+		 * (normally a single-dash) but typed two dashes instead.
+		 * note: even if we find a short option here, we do not
+		 * attempt to unbundle in this case */
+		if ((s->s && opt_char_match(s, cur)) ||
+					(s->l && opt_abbrev_match(s, cur))) {
+			if (found && found->id != s->id)
+				goto pass_through;
+			found = s;
+		}
+	}
+
+	/* ok, try harder, based on tokenization on '-' */
+	if (!found && getenv("GIT_ABBREV_HARDER")) {
+		for (i = 0; ost[i].l || ost[i].s; i++) {
+			s = &(ost[i]);
+			if (s->l && opt_token_match(s,cur)) {
+				if (found && found->id != s->id)
+					goto pass_through;
+				found = s;
+			}
+		}
+	}
+	/* should I add a strstr() matcher here for the desperate? */
+
+	/* rewrite the abbreviated switch in it's unabbreviated form: */
+	if (found) {
+		char *tmp;
+		size_t len = 3 + strlen(cur); /* --cur=potential_args\0 */
+		size_t l_len;
+
+		/* favor long options: */
+		l_len = found->l ? strlen(found->l) : 0;
+		tmp = xcalloc(len + l_len, 1); /* don't free this */
+		tmp[0] = '-';
+
+		if (found->l) {
+			tmp[1] = '-';
+			memcpy(tmp + 2, found->l, l_len);
+		} else
+			tmp[1] = found->s;
+		if (found->arg_fl) {
+			const char *c;
+			if ((c = strchr(cur,'='))) {
+				/* skip '=' for short opts masquerading as
+				 * long opts: --S=foo */
+				if (!l_len) c++;
+				strcpy(tmp + 2 + l_len, c);
+			}
+		}
+
+		gi->argv[gi->pos] = tmp;
+		return push_one_opt(gi, found);
+	}
+
+pass_through:
+	if (gitopt_pass_through)
+		return GITOPT_NON_OPTION;
+	gitopt_errno = error("Unknown option: '%s'",gi->argv[gi->pos]);
+	return GITOPT_ERROR;
+}
+
+static int opt_complete_match(const struct opt_spec *s, const char *p)
+{
+	if (s->arg_fl) {
+		size_t len = strlen(s->l);
+
+		return (!strncmp(s->l,p,len) &&
+				(p[len] == '\0' || (p[len] == '=')));
+	}
+	return !strcmp(s->l,p);
+}
+
+int gitopt_verify_b_args(const struct exec_args *b)
+{
+	const char **arg;
+
+	for (arg = b->argv; *arg; arg++) {
+		/* anything goes after a double dash */
+		if (!memcmp("--",*arg,3))
+			return 1;
+		if (*arg[0] == '-')
+			return 0;
+	}
+	return 1;
+}
+
+void gitopt_iter_setup(struct gitopt_iterator *gi,
+			const int argc, const char **argv)
+{
+	gi->upos = NULL;
+	gi->pos = 1;
+	gi->ea = new_exec_args(4); /* most we currently have is ARG_THREE */
+	gi->b = new_exec_args(argc);
+	gi->b->argc = 0;
+	gi->argc = argc;
+	gi->argv = argv;
+}
+
+int gitopt_iter_parse(struct gitopt_iterator *gi,
+			const struct opt_spec *ost)
+{
+	const char *c = gi->argv[gi->pos];
+	const struct opt_spec *s;
+	int i;
+
+	gitopt_errno = 0;
+	if (!c) return 0;
+	nil_exec_args(gi->ea);
+
+	if (!gitopt_dd_seen && !memcmp("--",c,3)) {
+		gitopt_dd_seen = 1;
+		return GITOPT_DD;
+	}
+	if (gitopt_dd_seen)
+		return GITOPT_NON_OPTION;
+	if (!memcmp("--",c,2)) {	/* long options */
+		c += 2;
+		for (i = 0; ost[i].l || ost[i].s; i++) {
+			s = &(ost[i]);
+			if (!s->l || !opt_complete_match(s, c)) continue;
+			return push_one_opt(gi, s);
+		}
+		/* undefined --option: */
+		return fallback_long(gi, ost, c);
+	}
+	if ((c[0] == '-') && (c[1] != '-')) { /* short option */
+		c++;
+		for (i = 0; ost[i].l || ost[i].s; i++) {
+			s = &(ost[i]);
+			if (!s->s) continue;
+			if (isspace(s->s) && (*c == '\0' || isdigit(*c))) {
+				/* special case for -<num> */
+				return push_one_opt(gi, s);
+			}
+			if (s->s != *c) continue;
+			if ((c[1] != '\0') && (c[1] != '='))
+				return unbundle_iter(gi, ost);
+			return push_one_opt(gi, s);
+		}
+		/* undefined: */
+		if (gitopt_pass_through)
+			return GITOPT_NON_OPTION;
+		return GITOPT_ERROR;
+	}
+	return GITOPT_NON_OPTION;
+}
+
+void gitopt_iter_next(struct gitopt_iterator *gi)
+{
+	if (!gi->upos || !gi->upos[0])
+		gi->pos++;
+}
+
+static int gitopt_parse_ost_split(struct exec_args *a, struct exec_args *b,
+			const struct opt_spec *ost,
+			const int argc, const char **argv)
+{
+	struct gitopt_iterator gi;
+
+	gitopt_dd_seen = 0;
+	b->argc = 0;
+	a->argv[0] = argv[0];
+	a->argc = 1;
+
+	gitopt_iter_setup(&gi, argc, argv);
+	for (; gi.pos < argc; gitopt_iter_next(&gi)) {
+		switch (gitopt_iter_parse(&gi, ost)) {
+		case GITOPT_ERROR:
+		case GITOPT_DD:
+			if (!gitopt_pass_through)
+				break;
+		case GITOPT_NON_OPTION:
+			b->argv[b->argc++] = gi.argv[gi.pos];
+			break;
+		default:
+			combine_exec_args(a, gi.ea);
+		}
+	}
+
+	free_exec_args(gi.ea);
+	free_exec_args(gi.b);
+	return gitopt_errno;
+}
+
+/* You should really only use this in git (wrapper) and test-gitopt: */
+struct exec_args *gitopt_parse_ost(const struct opt_spec *ost,
+			const int argc, const char **argv)
+{
+	struct exec_args *a = new_exec_args(argc); /* argv[0] and options: */
+	struct exec_args *b = new_exec_args(argc); /* non-option args */
+
+	gitopt_pass_through = 1;
+
+	if (gitopt_parse_ost_split(a, b, ost, argc, argv) < 0)
+		die("gitopt argument parsing failed");
+	combine_exec_args(a, b);
+	free_exec_args(b);
+
+	return a;
+}
+
+void free_exec_args(struct exec_args *ea)
+{
+	free(ea->argv);
+	free(ea);
+}
+
+struct opt_spec *combine_opt_spec(const struct opt_spec *a,
+					const struct opt_spec *b)
+{
+	struct opt_spec *rv, *tmp;
+	size_t a_size = 0, b_size = 0;
+
+	while (a[a_size].l || a[a_size].s) a_size++;
+	while (b[b_size].l || b[b_size].s) b_size++;
+
+	tmp = rv = xmalloc( (a_size + b_size + 1) * sizeof(*a) );
+
+	while (a->s || a->l) memcpy(tmp++, a++, sizeof(*a));
+	while (b->s || b->l) memcpy(tmp++, b++, sizeof(*b));
+
+	memcpy(tmp, b, sizeof(*b));
+
+	return rv;
+}
+
diff --git a/gitopt.h b/gitopt.h
new file mode 100644
index 0000000..4108cf5
--- /dev/null
+++ b/gitopt.h
@@ -0,0 +1,120 @@
+#ifndef GITOPT_H
+#define GITOPT_H
+
+/* gitopt_* functions will return this structure
+ * the elements in this struct can then be treated just
+ * like their counterparts from main(). */
+struct exec_args {
+	const char **argv;
+	int argc;
+};
+
+#define GITOPT_DIFF_BASE		64
+#define GITOPT_SR_BASE			128
+
+enum gitopt_status {
+	GITOPT_DD = 253,
+	GITOPT_NON_OPTION,
+	GITOPT_ERROR
+};
+
+/* @l: long option string (without the leading "--")
+ *
+ * @s: single option char, ' ' has a special meaning for accepting a
+ * single '-' (dash), which can also accept an integer argument This is
+ * for things like "-5" => "--max-count=5" or "-" => "--stdin"
+ *
+ * @rewrite_fmt: rewrite the passed argument(s) (if any) into this
+ * (*printf) style string.  Only a single %s can be accepted and handled
+ * by the default fn() handlers included in gitopt.
+ *
+ * Do not use this if you need to use more than one "%s", you'll need to
+ * define and use a custom fn().  rewrite_fmt is only intended for
+ * the common argument rewriting cases.
+ *
+ * If rewrite_fmt has a "%s", " %s" or "=%s" in it, it will be stripped
+ * out if no arguments are passed to it (this can be the case where
+ * fn() (see below) is defined to optional_arg).
+ *
+ * Any single space between non-space characters will be interpreted as
+ * break in the option and the options will be split out into seperate
+ * elements in argv.
+ *
+ * @arg_fl: argument flags, see ARG_* #defines
+ *
+ * @id: unique identifier to return, must be non-zero and < 64
+ */
+struct opt_spec {
+	const char *l;
+	const char s;
+	const char *rewrite_fmt;
+	const unsigned int arg_fl;
+	const int id;
+};
+
+/* internal use: */
+#define ARG_IS_INT	0x08
+#define ARG_IS_OPT	0x10
+
+/* use these for opt_spec flags: */
+#define ARG_NIL		0x00
+#define ARG_ONE		0x01
+#define ARG_TWO		0x02	/* not really supported yet */
+#define ARG_THREE	0x04	/* not really supported yet */
+#define ARG_TRE		ARG_THREE
+#define ARG_INT		(ARG_ONE | ARG_IS_INT)
+#define ARG_OPT		(ARG_ONE | ARG_IS_OPT)
+#define ARG_OPTINT	(ARG_ONE | ARG_IS_OPT | ARG_IS_INT)
+
+extern int gitopt_pass_through;
+extern int gitopt_dd_seen;	/* double-dash seen flag */
+
+struct exec_args *new_exec_args(const int argc);
+void free_exec_args(struct exec_args *ea);
+struct exec_args *nil_exec_args(struct exec_args *ea);
+
+/* You should really only use this in the git wrapper or tests: */
+struct exec_args *gitopt_parse_ost(const struct opt_spec *ost,
+			const int argc, const char **argv);
+
+/* used for debugging */
+static inline void dump_ea(const char *pfx, struct exec_args *ea)
+{
+	const char **arg;
+	int i = 0;
+	for (arg = ea->argv; *arg; arg++)
+		fprintf(stderr,"[%d] %s: %s\n",i++,pfx,*arg);
+}
+
+struct gitopt_iterator {
+	struct exec_args *ea;	/* tmp, for passing --opt args */
+	struct exec_args *b;	/* unrecognized arguments */
+	const char *upos;	/* unbundle position */
+	const char **argv;
+	int argc;
+	int pos;		/* argc position */
+};
+
+void gitopt_iter_setup(struct gitopt_iterator *gi,
+			const int argc, const char **argv);
+int gitopt_iter_parse(struct gitopt_iterator *gi,
+			const struct opt_spec *ost);
+void gitopt_iter_next(struct gitopt_iterator *gi);
+
+static inline void gitopt_iter_done(struct gitopt_iterator *gi)
+{
+	free_exec_args(gi->ea);
+}
+
+/* returns a newly allocated opt_spec struct, can be free()-ed: */
+struct opt_spec *combine_opt_spec(const struct opt_spec *a,
+					const struct opt_spec *b);
+
+struct gitopt_extra {
+	const struct opt_spec *ost;
+	int (*opt_handler)(struct gitopt_iterator *gi,
+			const int id, void *args);
+	void *args;
+};
+
+#endif /* GITOPT_H */
diff --git a/gitopt/git_wrapper.h b/gitopt/git_wrapper.h
new file mode 100644
index 0000000..5f27bf4
--- /dev/null
+++ b/gitopt/git_wrapper.h
@@ -0,0 +1,45 @@
+/* opt_spec table mappings for the git.c wrapper */
+
+#include "../gitopt.h"
+#include "../gitopt/sh.h"
+
+static const struct opt_spec ost_null[] = { { 0, 0 } };
+
+static const struct opt_spec_map {
+	const char *cmd;
+	const struct opt_spec *ost;
+} opt_specs[] = {
+	{ "checkout",		ost_checkout },
+	{ "commit",		ost_commit },
+	{ "am",			ost_am},
+};
+
+static const struct opt_spec *find_cmd_ost(const int argc, const char **argv)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(opt_specs); i++) {
+		const char *cmd = opt_specs[i].cmd;
+		if (strcmp(cmd, *argv))
+			continue;
+		return opt_specs[i].ost;
+	}
+	return NULL;
+}
+
+static struct exec_args *gitopt_parse_git(const int argc, const char **argv)
+{
+	const struct opt_spec *ost;
+
+	if (!strcmp(argv[0],"help") || !strcmp(argv[0],"version") ||
+				!(ost = find_cmd_ost(argc, argv))) {
+		struct exec_args *ea = new_exec_args(argc);
+		int i;
+
+		for (i = 0; i <= argc; i++) /* argv[argc] == NULL */
+			ea->argv[i] = argv[i];
+
+		return ea;
+	}
+	return gitopt_parse_ost(ost, argc, argv);
+}
diff --git a/gitopt/sh.h b/gitopt/sh.h
new file mode 100644
index 0000000..0ce6620
--- /dev/null
+++ b/gitopt/sh.h
@@ -0,0 +1,45 @@
+#ifndef GITOPT_SH_H
+#define GITOPT_SH_H
+
+/* opt_spec tables for some git programs written in shell that don't
+ * have too many options */
+
+static const struct opt_spec ost_am[] = {
+	{ "dotest",		'd',	0,		ARG_ONE,	0 },
+	{ "interactive",	'i',	0,		0,		0 },
+	{ "binary",		'b',	0,		0,		0 },
+	{ "3way",		'3',	0,		0,		0 },
+	{ "signoff",		's',	0,		0,		0 },
+	{ "utf8",		'u',	0,		0,		0 },
+	{ "keep",		'k',	0,		0,		0 },
+	{ "resolved",		'r',	0,		0,		0 },
+	{ "skip",		0,	0,		0,		0 },
+	{ "whitespace",		0,	0,		ARG_ONE,	0 },
+	{ 0, 0 }
+};
+
+static const struct opt_spec ost_checkout[] = {
+	{ 0,			'f',	0,		0,		0 },
+	{ 0,			'm',	0,		0,		0 },
+	{ 0,			'b',	"-b %s",	ARG_ONE,	0 },
+	{ 0, 0 }
+};
+
+static const struct opt_spec ost_commit[] = {
+	{ "file",		'F',	0,		ARG_ONE,	0 },
+	{ "all",		'a',	0,		0,		0 },
+	{ "author",		0,	0,		ARG_ONE,	0 },
+	{ "edit",		'e',	0,		0,		0 },
+	{ "include",		'i',	0,		0,		0 },
+	{ "only",		'o',	0,		0,		0 },
+	{ "message",		'm',	0,		ARG_ONE,	0 },
+	{ "no-verify",		'n',	0,		0,		0 },
+	{ "amend",		0,	0,		0,		0 },
+	{ "reedit",		'c',	0,		ARG_ONE,	0 },
+	{ "reuse-message",	'C',	0,		ARG_ONE,	0 },
+	{ "signoff",		's',	0,		0,		0 },
+	{ "verbose",		'v',	0,		0,		0 },
+	{ 0, 0 }
+};
+
+#endif /* GITOPT_SH_H */
diff --git a/t/t0200-gitopt.sh b/t/t0200-gitopt.sh
new file mode 100755
index 0000000..7e140a0
--- /dev/null
+++ b/t/t0200-gitopt.sh
@@ -0,0 +1,295 @@
+#!/bin/sh
+#
+# Copyright (c) 2006 Eric Wong
+#
+
+test_description='gitopt command-line pa{ss,rs}er'
+
+. ./test-lib.sh
+
+cat > expect <<EOF
+00 'commit'
+01 '(null)'
+EOF
+test_expect_success 'single command' \
+	'test-gitopt commit > output && cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-a'
+02 '-s'
+03 '(null)'
+EOF
+test_expect_success 'simple command with switches' \
+	'test-gitopt commit -a -s > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-a'
+02 '-s'
+03 '(null)'
+EOF
+test_expect_success 'command with bundled switches' \
+	'test-gitopt commit -as > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-a'
+02 '-s'
+03 '-mhello world'
+04 '(null)'
+EOF
+test_expect_success 'bundle with args for last (no space)' \
+	'test-gitopt commit -asm"hello world" > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-a'
+02 '-s'
+03 '-m'
+04 'hello world'
+05 '(null)'
+EOF
+test_expect_success 'unbundle with args (space)' \
+	'test-gitopt commit -asm "hello world" > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-a'
+02 '-s'
+03 '-m'
+04 'hello world'
+05 'file1'
+06 'file2'
+07 '(null)'
+EOF
+test_expect_success 'unbundle and reorder switches and command w/args' \
+	'test-gitopt commit file1 file2 -asm "hello world" > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-a'
+02 '-s'
+03 '--edit'
+04 'file2'
+05 '--'
+06 'file1'
+07 '-as'
+08 '--all'
+09 '(null)'
+EOF
+test_expect_success 'reorder up to and pass-through "--"' \
+	'test-gitopt commit -as file2 --edit -- file1 -as --all > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '-s'
+02 '-m'
+03 'message'
+04 '--'
+05 'file1'
+06 'file2'
+07 '(null)'
+EOF
+test_expect_success 'reorder up to and pass-through "--"' \
+	'test-gitopt commit -sm message -- file1 file2 > output &&
+	cmp expect output'
+
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '--find-copies-harder'
+02 '(null)'
+EOF
+test_expect_success 'abbreviation finder (prefix)' \
+	'test-gitopt whatchanged --find-c > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '--patch-with-raw'
+02 '(null)'
+EOF
+test_expect_success 'abbreviation finder (substring on "-" token)' \
+	'GIT_ABBREV_HARDER=1 test-gitopt whatchanged --raw > output &&
+	 cmp expect output'
+
+# we assume unknown switches that cannot be resolved to a single known
+# switch to just be an new argument we do not know about, so we pass
+# it to the underlying command
+cat > expect <<EOF
+00 'whatchanged'
+01 '--zzzz'
+02 '(null)'
+EOF
+test_expect_success 'ambiguous abbreviation (pass-through)' \
+	'test-gitopt whatchanged --zzzz > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '--diff-filter=MCT'
+02 'file1'
+03 '(null)'
+EOF
+test_expect_success 'rewrite on long argument' \
+	'test-gitopt whatchanged file1 --diff-filter MCT > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-Shello'
+02 '(null)'
+EOF
+test_expect_success 'rewrite on short argument (#1)' \
+	'test-gitopt whatchanged -Shello > output &&
+	 cmp expect output'
+test_expect_success 'rewrite on short argument (#2)' \
+	'test-gitopt whatchanged -S hello > output &&
+	 cmp expect output'
+test_expect_success 'rewrite on --short argument (#3)' \
+	'test-gitopt whatchanged --S hello > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-Shello'
+02 '(null)'
+EOF
+test_expect_success 'rewrite on short argument (leading "=" arg) (#1)' \
+	'test-gitopt whatchanged --S=hello > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-C20'
+02 '-M'
+03 '(null)'
+EOF
+test_expect_success 'pass optional arg (#1)' \
+	'test-gitopt whatchanged -C20 -M > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-C'
+02 '--find-copies-harder'
+03 '(null)'
+EOF
+test_expect_success 'detect optional arg bogus (#1)' \
+	'test-gitopt whatchanged -C --find-copies-harder > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-C20'
+02 '(null)'
+EOF
+test_expect_success 'pass optional arg (#2)' \
+	'test-gitopt whatchanged -C20 > output &&
+	 cmp expect output'
+# test_expect_success 'pass optional arg (#3)' \
+	# 'test-gitopt whatchanged -C=20 > output &&
+	 # cmp expect output'
+
+cat > expect <<EOF
+00 'checkout'
+01 '-b'
+02 'newbranch'
+03 '(null)'
+EOF
+test_expect_success 'rewrite short split arg (#1)' \
+	'test-gitopt checkout -bnewbranch > output &&
+	 cmp expect output'
+# test_expect_success 'rewrite short split arg (#2)' \
+	# 'test-gitopt checkout --b=newbranch > output &&
+	 # cmp expect output'
+test_expect_success 'rewrite short sanity check' \
+	'test-gitopt checkout -b newbranch > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'log'
+01 '--default'
+02 'dunno'
+03 '--all'
+04 '(null)'
+EOF
+test_expect_success 'rewrite long split arg' \
+	'test-gitopt log --default=dunno --all > output &&
+	 cmp expect output'
+test_expect_success 'rewrite long sanity check' \
+	'test-gitopt log --default dunno --all > output &&
+	 cmp expect output'
+
+cat > expect <<EOF
+00 'log'
+01 '--max-count=56'
+02 '(null)'
+EOF
+test_expect_success 'rewrite -<num> => --max-count=<num>' \
+	'test-gitopt log -56 > output && cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-u'
+02 '-l20'
+03 '-p'
+04 '-Spicktoken'
+05 '(null)'
+EOF
+test_expect_success 'bundle options with integer args mixed in' \
+	'test-gitopt whatchanged -ul20pSpicktoken > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-u'
+02 '-C20'
+03 '-p'
+04 '(null)'
+EOF
+test_expect_success 'bundle options with optional integer args used' \
+	'test-gitopt whatchanged -uC20p > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-u'
+02 '-C'
+03 '-p'
+04 '(null)'
+EOF
+test_expect_success 'bundle options with optional integer args not used' \
+	'test-gitopt whatchanged -uCp > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'whatchanged'
+01 '-C'
+02 '20'
+03 '(null)'
+EOF
+test_expect_success 'optional integer arg switch must be attached' \
+	'test-gitopt whatchanged -C 20 > output &&
+	cmp expect output'
+
+cat > expect <<EOF
+00 'commit'
+01 '--message'
+02 'hello world'
+03 '-s'
+04 'file'
+05 '(null)'
+EOF
+test_expect_success 'long option argument parsing when short option can work' \
+	'test-gitopt commit --message "hello world" -s file > output &&
+	cmp expect output'
+
+test_done
diff --git a/test-gitopt.c b/test-gitopt.c
new file mode 100644
index 0000000..2692e2b
--- /dev/null
+++ b/test-gitopt.c
@@ -0,0 +1,112 @@
+
+#include "git-compat-util.h"
+#include "gitopt.h"
+#include "gitopt/sh.h"
+
+#define _rev \
+{ "max-count",		'n',	"--max-count=%s",	ARG_INT,	0 }, \
+{ 0,			' ',	"--max-count=%s",	ARG_INT,	0 }, \
+{ "max-age",		0,	"--max-age=%s",		ARG_INT,	0 }, \
+{ "min-age",		0,	"--min-age=%s",		ARG_INT,	0 }, \
+{ "since",		0,	"--since=%s",		ARG_ONE,	0 }, \
+{ "after",		0,	"--after=%s",		ARG_ONE,	0 }, \
+{ "before",		0,	"--before=%s",		ARG_ONE,	0 }, \
+{ "until",		0,	"--until=%s",		ARG_ONE,	0 }, \
+{ "all",		0,	0,			0,		0 }, \
+{ "not",		0,	0,			0,		0 }, \
+{ "default",		0,	"--default %s",		ARG_ONE,	0 }, \
+{ "topo-order",		0,	0,			0,		0 }, \
+{ "date-order",		0,	0,			0,		0 }, \
+{ "parents",		0,	0,			0,		0 }, \
+{ "dense",		0,	0,			0,		0 }, \
+{ "sparse",		0,	0,			0,		0 }, \
+{ "remove-empty",	0,	0,			0,		0 }, \
+{ "no-merges",		0,	0,			0,		0 }, \
+{ "boundary",		0,	0,			0,		0 }, \
+{ "objects",		0,	0,			0,		0 }, \
+{ "objects-edge",	0,	0,			0,		0 }, \
+{ "unpacked",		0,	0,			0,		0 }, \
+{ 0,			'r',	0,			0,		0 }, \
+{ 0,			't',	0,			0,		0 }, \
+{ 0,			'm',	0,			0,		0 }, \
+{ 0,			'c',	0,			0,		0 }, \
+{ "cc",			0,	0,			0,		0 }, \
+{ 0,			'v',	0,			0,		0 }, \
+{ "pretty",		0,	"--pretty=%s",		ARG_ONE,	0 }, \
+{ "root",		0,	0,			0,		0 }, \
+{ "no-commit-id",	0,	0,			0,		0 }, \
+{ "always",		0,	0,			0,		0 }, \
+{ "no-abbrev",		0,	0,			0,		0 }, \
+{ "abbrev",		0,	0,			0,		0 }, \
+{ "abbrev-commit",	0,	0,			0,		0 }, \
+{ "full-diff",		0,	0,			0,		0 }, \
+
+#define _diff \
+{ 0,			'u',	0,			0,		0 }, \
+{ 0,			'p',	0,			0,		0 }, \
+{ "patch-with-raw",	0,	0,			0,		0 }, \
+{ "stat",		0,	0,			0,		0 }, \
+{ "patch-with-stat",	0,	0,			0,		0 }, \
+{ 0,			'z',	0,			0,		0 }, \
+{ 0,			'l',	"-l%s",			ARG_INT,	0 }, \
+{ "full-index",		0,	0,			0,		0 }, \
+{ "name-only",		0,	0,			0,		0 }, \
+{ "name-status",	0,	0,			0,		0 }, \
+{ 0,			'R',	0,			0,		0 }, \
+{ 0,			'S',	"-S%s",			ARG_ONE,	0 }, \
+{ 0,			's',	0,			0,		0 }, \
+{ 0,			'O',	"-O%s",			ARG_ONE,	0 }, \
+{ "diff-filter",	0,	"--diff-filter=%s",	ARG_ONE,	0 }, \
+{ "pickaxe-all",	0,	0,			0,		0 }, \
+{ "pickaxe-regex",	0,	0,			0,		0 }, \
+{ 0,			'B',	"-B%s",			ARG_OPTINT,	0 }, \
+{ 0,			'M',	"-M%s",			ARG_OPTINT,	0 }, \
+{ 0,			'C',	"-C%s",			ARG_OPTINT,	0 }, \
+{ "find-copies-harder",	0,	0,			0,		0 }, \
+{ "abbrev",		0,	"--abbrev=%s",		ARG_OPT,	0 }, \
+
+#define end {0,0}
+
+static const struct opt_spec ost_log[] = { _rev end };
+static const struct opt_spec ost_rev_list[] = { _rev end };
+static const struct opt_spec ost_whatchanged[] = { _diff _rev end };
+static const struct opt_spec ost_show[] = { _diff _rev end };
+
+static const struct opt_spec_map {
+	const char *cmd;
+	const struct opt_spec *ost;
+} opt_specs[] = {
+	{ "checkout",		ost_checkout },
+	{ "commit",		ost_commit },
+	{ "log",		ost_log },
+	{ "rev-list",		ost_rev_list },
+	{ "show",		ost_show },
+	{ "whatchanged",	ost_whatchanged },
+};
+
+
+int main(int argc, const char **argv, char **envp)
+{
+	int i;
+	struct exec_args *ea;
+	const struct opt_spec *ost = NULL;
+
+	if (!argv[1])
+		usage("test-gitopt [<options>] <command> [<options>]");
+
+	for (i = 0; i < ARRAY_SIZE(opt_specs); i++) {
+		if (!strcmp(argv[1], opt_specs[i].cmd)) {
+			ost = opt_specs[i].ost;
+			break;
+		}
+	}
+	if (!ost)
+		usage("test-gitopt [<options>] <command> [<options>]");
+
+	ea = gitopt_parse_ost(ost, argc - 1, argv + 1);
+
+	for (i = 0; i <= ea->argc; i++)
+		printf("%02d '%s'\n", i, ea->argv[i] ? ea->argv[i] : "(null)");
+
+	return 0;
+}
-- 
1.3.2.g102e322
Eric Wong· May 14, 2006, 15:19 UTC · re: Eric Wong · lore

[PATCH 2/5] gitopt: convert ls-files, ls-tree, update-index

All of these are pretty straighforward conversions
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 ls-files.c     |  184 +++++++++++++++++++++-------------------
 ls-tree.c      |   82 ++++++++++++------
 update-index.c |  257 ++++++++++++++++++++++++++++++--------------------------
 3 files changed, 287 insertions(+), 236 deletions(-)
962175b2be70eab14997a10ebb9c7cd719d72c1b
Show changes to 3 files +287 −236

ls-files.c, ls-tree.c, update-index.c

diff --git a/ls-files.c b/ls-files.c
index 4a4af1c..9f4d328 100644
--- a/ls-files.c
+++ b/ls-files.c
@@ -10,6 +10,7 @@ #include <fnmatch.h>
 
 #include "cache.h"
 #include "quote.h"
+#include "gitopt.h"
 
 static int abbrev = 0;
 static int show_deleted = 0;
@@ -648,133 +649,140 @@ static const char ls_files_usage[] =
 	"[ --exclude-per-directory=<filename> ] [--full-name] [--abbrev] "
 	"[--] [<file>]*";
 
+enum ls_files_ids {
+	opt_z = 1, opt_v, opt_t, opt_c, opt_d, opt_m, opt_o, opt_i, opt_s,
+	opt_k, opt_directory, opt_no_empty_directory,
+	opt_u, opt_x, opt_X, opt_exclude_per_dir,
+	opt_abbrev, opt_full_name, opt_error_unmatch
+};
+
+static const struct opt_spec ls_files_ost[] = {
+	{ 0,			'z',	0,	0,	opt_z },
+	{ 0,			'v',	0,	0,	opt_v },
+	{ 0,			't',	0,	0,	opt_t },
+	{ "cached",		'c',	0,	0,	opt_c },
+	{ "deleted",		'd',	0,	0,	opt_d },
+	{ "modified",		'm',	0,	0,	opt_m },
+	{ "others",		'o',	0,	0,	opt_o },
+	{ "ignored",		'i',	0,	0,	opt_i },
+	{ "stage",		's',	0,	0,	opt_s },
+	{ "killed",		'k',	0,	0,	opt_k },
+	{ "directory",		0,	0,	0,	opt_directory },
+	{ "no-empty-directory",	0,	0,	0,	opt_no_empty_directory},
+	{ "unmerged",		'u',	0,	0,	opt_u },
+	{ "exclude",		'x',	0,	ARG_ONE,	opt_x },
+	{ "exclude-from",	'X',	0,	ARG_ONE,	opt_X },
+	{ "exclude-per-directory",0,	0,	ARG_ONE, opt_exclude_per_dir },
+	{ "full-name",		0,	0,	0,	opt_full_name },
+	{ "error-unmatch",	0,	0,	0,	opt_error_unmatch },
+	{ "abbrev",		0,	0,	ARG_OPTINT, opt_abbrev },
+	{ 0, 0 }
+};
+
 int main(int argc, const char **argv)
 {
 	int i;
 	int exc_given = 0;
+	struct gitopt_iterator gi;
+	struct exec_args *ea, *b;
 
 	prefix = setup_git_directory();
 	if (prefix)
 		prefix_offset = strlen(prefix);
 	git_config(git_default_config);
 
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-
-		if (!strcmp(arg, "--")) {
-			i++;
-			break;
-		}
-		if (!strcmp(arg, "-z")) {
+	gitopt_iter_setup(&gi, argc, argv);
+	ea = gi.ea;
+	b = gi.b;
+	for (; (i = gitopt_iter_parse(&gi, ls_files_ost));
+				gitopt_iter_next(&gi)) {
+		switch (i) {
+		case opt_z:
 			line_terminator = 0;
-			continue;
-		}
-		if (!strcmp(arg, "-t") || !strcmp(arg, "-v")) {
+			break;
+		case opt_v:
+			show_valid_bit = 1; /* fall through */
+		case opt_t:
 			tag_cached = "H ";
 			tag_unmerged = "M ";
 			tag_removed = "R ";
 			tag_modified = "C ";
 			tag_other = "? ";
 			tag_killed = "K ";
-			if (arg[1] == 'v')
-				show_valid_bit = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-c") || !strcmp(arg, "--cached")) {
+			break;
+		case opt_c:
 			show_cached = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-d") || !strcmp(arg, "--deleted")) {
+			break;
+		case opt_d:
 			show_deleted = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-m") || !strcmp(arg, "--modified")) {
+			break;
+		case opt_m:
 			show_modified = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-o") || !strcmp(arg, "--others")) {
+			break;
+		case opt_o:
 			show_others = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-i") || !strcmp(arg, "--ignored")) {
+			break;
+		case opt_i:
 			show_ignored = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-s") || !strcmp(arg, "--stage")) {
+			break;
+		case opt_s:
 			show_stage = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-k") || !strcmp(arg, "--killed")) {
+			break;
+		case opt_k:
 			show_killed = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--directory")) {
+			break;
+		case opt_directory:
 			show_other_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--no-empty-directory")) {
+			break;
+		case opt_no_empty_directory:
 			hide_empty_directories = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-u") || !strcmp(arg, "--unmerged")) {
+			break;
+		case opt_u:
 			/* There's no point in showing unmerged unless
 			 * you also show the stage information.
 			 */
 			show_stage = 1;
 			show_unmerged = 1;
-			continue;
-		}
-		if (!strcmp(arg, "-x") && i+1 < argc) {
-			exc_given = 1;
-			add_exclude(argv[++i], "", 0, &exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strncmp(arg, "--exclude=", 10)) {
-			exc_given = 1;
-			add_exclude(arg+10, "", 0, &exclude_list[EXC_CMDL]);
-			continue;
-		}
-		if (!strcmp(arg, "-X") && i+1 < argc) {
+			break;
+		case opt_x:
 			exc_given = 1;
-			add_excludes_from_file(argv[++i]);
-			continue;
-		}
-		if (!strncmp(arg, "--exclude-from=", 15)) {
+			add_exclude(ea->argv[0], "", 0, &exclude_list[EXC_CMDL]);
+			break;
+		case opt_X:
 			exc_given = 1;
-			add_excludes_from_file(arg+15);
-			continue;
-		}
-		if (!strncmp(arg, "--exclude-per-directory=", 24)) {
+			add_excludes_from_file(ea->argv[0]);
+			break;
+		case opt_exclude_per_dir:
 			exc_given = 1;
-			exclude_per_dir = arg + 24;
-			continue;
-		}
-		if (!strcmp(arg, "--full-name")) {
+			exclude_per_dir = ea->argv[0];
+			break;
+		case opt_full_name:
 			prefix_offset = 0;
-			continue;
-		}
-		if (!strcmp(arg, "--error-unmatch")) {
+			break;
+		case opt_error_unmatch:
 			error_unmatch = 1;
-			continue;
-		}
-		if (!strncmp(arg, "--abbrev=", 9)) {
-			abbrev = strtoul(arg+9, NULL, 10);
-			if (abbrev && abbrev < MINIMUM_ABBREV)
-				abbrev = MINIMUM_ABBREV;
-			else if (abbrev > 40)
-				abbrev = 40;
-			continue;
-		}
-		if (!strcmp(arg, "--abbrev")) {
-			abbrev = DEFAULT_ABBREV;
-			continue;
-		}
-		if (*arg == '-')
+			break;
+		case opt_abbrev:
+			if (ea->argc == 1)
+				abbrev = DEFAULT_ABBREV;
+			else {
+				if (abbrev && abbrev < MINIMUM_ABBREV)
+					abbrev = MINIMUM_ABBREV;
+				else if (abbrev > 40)
+					abbrev = 40;
+			}
+			break;
+		case GITOPT_DD:
+			break;
+		case GITOPT_NON_OPTION:
+			b->argv[b->argc++] = argv[gi.pos];
+			break;
+		case GITOPT_ERROR:
 			usage(ls_files_usage);
-		break;
+		}
 	}
 
-	pathspec = get_pathspec(prefix, argv + i);
+	pathspec = get_pathspec(prefix, b->argv);
 
 	/* Verify that the pathspec matches the prefix */
 	if (pathspec)
diff --git a/ls-tree.c b/ls-tree.c
index f2b3bc1..3de85bc 100644
--- a/ls-tree.c
+++ b/ls-tree.c
@@ -7,6 +7,7 @@ #include "cache.h"
 #include "blob.h"
 #include "tree.h"
 #include "quote.h"
+#include "gitopt.h"
 
 static int line_termination = '\n';
 #define LS_RECURSIVE 1
@@ -84,56 +85,79 @@ static int show_tree(unsigned char *sha1
 	return retval;
 }
 
+enum ls_tree_ids {
+	opt_z = 1, opt_r, opt_d, opt_t,
+	opt_name_only_status, opt_full_name, opt_abbrev
+};
+
+static const struct opt_spec ls_tree_ost[] = {
+	{ 0,			'z',	0,	0,	opt_z },
+	{ 0,			'r',	0,	0,	opt_r },
+	{ 0,			'd',	0,	0,	opt_d },
+	{ 0,			't',	0,	0,	opt_t },
+	{ "name-only",		0,	0,	0,	opt_name_only_status },
+	{ "name-status",	0,	0,	0,	opt_name_only_status },
+	{ "full-name",		0,	0,	0,	opt_full_name },
+	{ "abbrev",		0,	0,	ARG_OPTINT,	opt_abbrev },
+	{ 0, 0 }
+};
+
 int main(int argc, const char **argv)
 {
 	unsigned char sha1[20];
 	struct tree *tree;
+	struct exec_args *b, *ea;
+	struct gitopt_iterator gi;
+	int i;
 
 	prefix = setup_git_directory();
 	git_config(git_default_config);
 	if (prefix && *prefix)
 		chomp_prefix = strlen(prefix);
-	while (1 < argc && argv[1][0] == '-') {
-		switch (argv[1][1]) {
-		case 'z':
+
+	gitopt_iter_setup(&gi, argc, argv);
+	ea = gi.ea;
+	b = gi.b;
+	for (; (i = gitopt_iter_parse(&gi, ls_tree_ost));
+				gitopt_iter_next(&gi)) {
+		switch (i) {
+		case opt_z:
 			line_termination = 0;
 			break;
-		case 'r':
+		case opt_r:
 			ls_options |= LS_RECURSIVE;
 			break;
-		case 'd':
+		case opt_d:
 			ls_options |= LS_TREE_ONLY;
 			break;
-		case 't':
+		case opt_t:
 			ls_options |= LS_SHOW_TREES;
 			break;
-		case '-':
-			if (!strcmp(argv[1]+2, "name-only") ||
-			    !strcmp(argv[1]+2, "name-status")) {
-				ls_options |= LS_NAME_ONLY;
-				break;
-			}
-			if (!strcmp(argv[1]+2, "full-name")) {
-				chomp_prefix = 0;
-				break;
-			}
-			if (!strncmp(argv[1]+2, "abbrev=",7)) {
-				abbrev = strtoul(argv[1]+9, NULL, 10);
+		case opt_name_only_status:
+			ls_options |= LS_NAME_ONLY;
+			break;
+		case opt_full_name:
+			chomp_prefix = 0;
+			break;
+		case opt_abbrev:
+			if (ea->argc == 1)
+				abbrev = DEFAULT_ABBREV;
+			else {
+				abbrev = strtoul(ea->argv[1], NULL, 10);
 				if (abbrev && abbrev < MINIMUM_ABBREV)
 					abbrev = MINIMUM_ABBREV;
 				else if (abbrev > 40)
 					abbrev = 40;
-				break;
-			}
-			if (!strcmp(argv[1]+2, "abbrev")) {
-				abbrev = DEFAULT_ABBREV;
-				break;
 			}
-			/* otherwise fallthru */
-		default:
+			break;
+		case GITOPT_DD:
+			break;
+		case GITOPT_NON_OPTION:
+			b->argv[b->argc++] = argv[gi.pos];
+			break;
+		case GITOPT_ERROR:
 			usage(ls_tree_usage);
 		}
-		argc--; argv++;
 	}
 	/* -d -r should imply -t, but -d by itself should not have to. */
 	if ( (LS_TREE_ONLY|LS_RECURSIVE) ==
@@ -142,10 +166,10 @@ int main(int argc, const char **argv)
 
 	if (argc < 2)
 		usage(ls_tree_usage);
-	if (get_sha1(argv[1], sha1))
-		die("Not a valid object name %s", argv[1]);
+	if (get_sha1(b->argv[0], sha1))
+		die("Not a valid object name %s", b->argv[0]);
 
-	pathspec = get_pathspec(prefix, argv + 2);
+	pathspec = get_pathspec(prefix, b->argv + 1);
 	tree = parse_tree_indirect(sha1);
 	if (!tree)
 		die("not a tree object");
diff --git a/update-index.c b/update-index.c
index 3d7e02d..f4e15d7 100644
--- a/update-index.c
+++ b/update-index.c
@@ -7,6 +7,7 @@ #include "cache.h"
 #include "strbuf.h"
 #include "quote.h"
 #include "tree-walk.h"
+#include "gitopt.h"
 
 /*
  * Default to not allowing changes to the list of files. The
@@ -645,6 +646,39 @@ static int do_reupdate(int ac, const cha
 	return 0;
 }
 
+enum update_index_ids {
+	opt_q = 1, opt_add, opt_replace, opt_remove, opt_unmerged,
+	opt_refresh, opt_really_refresh, opt_cacheinfo, opt_chmod,
+	opt_assume_unchanged, opt_no_assume_unchanged, opt_info_only,
+	opt_force_remove, opt_z, opt_stdin, opt_index_info, opt_unresolve,
+	opt_again, opt_ignore_missing, opt_verbose, opt_h
+};
+
+static const struct opt_spec update_index_ost[] = {
+	{ 0,			'q',	0,	0,	opt_q },
+	{ "add",		0,	0,	0,	opt_add },
+	{ "replace",		0,	0,	0,	opt_replace },
+	{ "remove",		0,	0,	0,	opt_remove },
+	{ "unmerged",		0,	0,	0,	opt_unmerged },
+	{ "refresh",		0,	0,	0,	opt_refresh },
+	{ "really-refresh",	0,	0,	0,	opt_really_refresh },
+	{ "cacheinfo",		0,	0,	ARG_TRE, opt_cacheinfo},
+	{ "chmod",		0,	"%s",	ARG_ONE, opt_chmod },
+	{ "assume-unchanged",	0,	0,	0,	opt_assume_unchanged },
+	{ "no-assume-unchanged",0,	0,	0,	opt_no_assume_unchanged},
+	{ "info-only",		0,	0,	0,	opt_info_only },
+	{ "force-remove",	0,	0,	0,	opt_force_remove },
+	{ 0,			'z',	0,	0,	opt_z },
+	{ "stdin",		0,	0,	0,	opt_stdin },
+	{ "index-info",		0,	0,	0,	opt_index_info },
+	{ "unresolve",		0,	0,	0,	opt_unresolve },
+	{ "again",		0,	0,	0,	opt_again },
+	{ "ignore-missing",	0,	0,	0,	opt_ignore_missing },
+	{ "verbose",		0,	0,	0,	opt_verbose },
+	{ "help",		'h',	0,	0,	opt_h },
+	{ 0, 0 }
+};
+
 int main(int argc, const char **argv)
 {
 	int i, newfd, entries, has_errors = 0, line_termination = '\n';
@@ -653,6 +687,10 @@ int main(int argc, const char **argv)
 	const char *prefix = setup_git_directory();
 	int prefix_length = prefix ? strlen(prefix) : 0;
 	char set_executable_bit = 0;
+	struct exec_args *ea;
+	struct gitopt_iterator gi;
+	unsigned char sha1[20];
+	unsigned int mode;
 
 	git_config(git_default_config);
 
@@ -664,126 +702,107 @@ int main(int argc, const char **argv)
 	if (entries < 0)
 		die("cache corrupted");
 
-	for (i = 1 ; i < argc; i++) {
-		const char *path = argv[i];
-
-		if (allow_options && *path == '-') {
-			if (!strcmp(path, "--")) {
-				allow_options = 0;
-				continue;
-			}
-			if (!strcmp(path, "-q")) {
-				quiet = 1;
-				continue;
-			}
-			if (!strcmp(path, "--add")) {
-				allow_add = 1;
-				continue;
-			}
-			if (!strcmp(path, "--replace")) {
-				allow_replace = 1;
-				continue;
-			}
-			if (!strcmp(path, "--remove")) {
-				allow_remove = 1;
-				continue;
-			}
-			if (!strcmp(path, "--unmerged")) {
-				allow_unmerged = 1;
-				continue;
-			}
-			if (!strcmp(path, "--refresh")) {
-				has_errors |= refresh_cache(0);
-				continue;
-			}
-			if (!strcmp(path, "--really-refresh")) {
-				has_errors |= refresh_cache(1);
-				continue;
-			}
-			if (!strcmp(path, "--cacheinfo")) {
-				unsigned char sha1[20];
-				unsigned int mode;
-
-				if (i+3 >= argc)
-					die("git-update-index: --cacheinfo <mode> <sha1> <path>");
-
-				if ((sscanf(argv[i+1], "%o", &mode) != 1) ||
-				    get_sha1_hex(argv[i+2], sha1) ||
-				    add_cacheinfo(mode, sha1, argv[i+3], 0))
-					die("git-update-index: --cacheinfo"
-					    " cannot add %s", argv[i+3]);
-				i += 3;
-				continue;
-			}
-			if (!strcmp(path, "--chmod=-x") ||
-			    !strcmp(path, "--chmod=+x")) {
-				if (argc <= i+1)
-					die("git-update-index: %s <path>", path);
-				set_executable_bit = path[8];
-				continue;
-			}
-			if (!strcmp(path, "--assume-unchanged")) {
-				mark_valid_only = MARK_VALID;
-				continue;
-			}
-			if (!strcmp(path, "--no-assume-unchanged")) {
-				mark_valid_only = UNMARK_VALID;
-				continue;
-			}
-			if (!strcmp(path, "--info-only")) {
-				info_only = 1;
-				continue;
-			}
-			if (!strcmp(path, "--force-remove")) {
-				force_remove = 1;
-				continue;
-			}
-			if (!strcmp(path, "-z")) {
-				line_termination = 0;
-				continue;
-			}
-			if (!strcmp(path, "--stdin")) {
-				if (i != argc - 1)
-					die("--stdin must be at the end");
-				read_from_stdin = 1;
-				break;
-			}
-			if (!strcmp(path, "--index-info")) {
-				if (i != argc - 1)
-					die("--index-info must be at the end");
-				allow_add = allow_replace = allow_remove = 1;
-				read_index_info(line_termination);
-				break;
-			}
-			if (!strcmp(path, "--unresolve")) {
-				has_errors = do_unresolve(argc - i, argv + i,
-							  prefix, prefix_length);
-				if (has_errors)
-					active_cache_changed = 0;
-				goto finish;
-			}
-			if (!strcmp(path, "--again")) {
-				has_errors = do_reupdate(argc - i, argv + i,
-							 prefix, prefix_length);
-				if (has_errors)
-					active_cache_changed = 0;
-				goto finish;
-			}
-			if (!strcmp(path, "--ignore-missing")) {
-				not_new = 1;
-				continue;
-			}
-			if (!strcmp(path, "--verbose")) {
-				verbose = 1;
-				continue;
-			}
-			if (!strcmp(path, "-h") || !strcmp(path, "--help"))
-				usage(update_index_usage);
-			die("unknown option %s", path);
+	gitopt_iter_setup(&gi, argc, argv);
+	ea = gi.ea;
+	for (; (i = gitopt_iter_parse(&gi, update_index_ost));
+				gitopt_iter_next(&gi)) {
+		switch (i) {
+		case opt_q:
+			quiet = 1;
+			break;
+		case opt_add:
+			allow_add = 1;
+			break;
+		case opt_replace:
+			allow_replace = 1;
+			break;
+		case opt_remove:
+			allow_remove = 1;
+			break;
+		case opt_unmerged:
+			allow_unmerged = 1;
+			break;
+		case opt_refresh:
+			has_errors |= refresh_cache(0);
+			break;
+		case opt_really_refresh:
+			has_errors |= refresh_cache(1);
+			break;
+		case opt_cacheinfo:
+			if (ea->argc != 4)
+				die("git-update-index: --cacheinfo "
+				    "<mode> <sha1> <path>");
+			if ((sscanf(ea->argv[1], "%o", &mode) != 1) ||
+			    get_sha1_hex(ea->argv[2], sha1) ||
+			    add_cacheinfo(mode, sha1, ea->argv[3], 0))
+				die("git-update-index: --cacheinfo"
+				    " cannot add %s", ea->argv[3]);
+			break;
+		case opt_chmod:
+			if (i+1 >= argc)
+				die("git-update-index: --chmod=%s <path>",
+								ea->argv[0]);
+			set_executable_bit = ea->argv[0][0];
+			break;
+		case opt_assume_unchanged:
+			mark_valid_only = MARK_VALID;
+			break;
+		case opt_no_assume_unchanged:
+			mark_valid_only = UNMARK_VALID;
+			break;
+		case opt_info_only:
+			info_only = 1;
+			break;
+		case opt_force_remove:
+			force_remove = 1;
+			break;
+		case opt_z:
+			line_termination = 0;
+			break;
+		case opt_stdin:
+			if (gi.pos != argc - 1)
+				die("--stdin must be at the end");
+			read_from_stdin = 1;
+			break;
+		case opt_index_info:
+			if (gi.pos != argc - 1)
+				die("--index-info must be at the end");
+			allow_add = allow_replace = allow_remove = 1;
+			read_index_info(line_termination);
+			break;
+		case opt_unresolve:
+			has_errors = do_unresolve(argc - gi.pos, argv + gi.pos,
+						  prefix, prefix_length);
+			if (has_errors)
+				active_cache_changed = 0;
+			goto finish;
+		case opt_again:
+			has_errors = do_reupdate(argc - gi.pos, argv + gi.pos,
+						 prefix, prefix_length);
+			if (has_errors)
+				active_cache_changed = 0;
+			goto finish;
+		case opt_ignore_missing:
+			not_new = 1;
+			break;
+		case opt_verbose:
+			verbose = 1;
+			break;
+		case opt_h:
+			usage(update_index_usage);
+			break;
+		case GITOPT_DD:
+			allow_options = 0;
+			break;
+		case GITOPT_NON_OPTION:
+			update_one(argv[gi.pos], prefix, prefix_length);
+			if (set_executable_bit)
+				chmod_path(set_executable_bit, argv[gi.pos]);
+			break;
+		case GITOPT_ERROR:
+			die("unknown option %s", argv[gi.pos]);
+			break;
 		}
-		update_one(path, prefix, prefix_length);
-		if (set_executable_bit)
-			chmod_path(set_executable_bit, path);
 	}
 	if (read_from_stdin) {
 		struct strbuf buf;
-- 
1.3.2.g102e322
Eric Wong· May 14, 2006, 15:19 UTC · re: Eric Wong · lore

[PATCH 3/5] gitopt: convert setup_revisions() and friends

	diff_opt_parse => diff_opt_handle
I've added --raw to diff_opt_handle() for consistency's sake

Lightly tested, some bugs in the original series of submitted patches were fixed wrt argument parsing for -C,-M,-B).

Passes all tests so that's a good sign.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 builtin-diff.c      |  152 +++++++-------
 builtin-log.c       |    7 -
 commit.c            |   14 +
 diff-files.c        |   25 +-
 diff-index.c        |   18 +-
 diff-stages.c       |   56 +++--
 diff-tree.c         |   15 +
 diff.c              |  183 ++++++++++++-----
 diff.h              |    7 -
 gitopt/diff-files.h |   45 ++++
 gitopt/diff-index.h |   22 ++
 gitopt/diff-tree.h  |   22 ++
 gitopt/diff.h       |   19 ++
 http-push.c         |    2 
 rev-list.c          |   51 +++--
 revision.c          |  542 +++++++++++++++++++++++++++------------------------
 revision.h          |    4 
 17 files changed, 704 insertions(+), 480 deletions(-)
 create mode 100644 gitopt/diff-files.h
 create mode 100644 gitopt/diff-index.h
 create mode 100644 gitopt/diff-tree.h
 create mode 100644 gitopt/diff.h
946397f743f363198d154efaa74d5cc5bd9d6aae
Show changes to 17 files +704 −480

builtin-diff.c, builtin-log.c, commit.c, diff-files.c, diff-index.c, diff-stages.c, diff-tree.c, diff.c, diff.h, gitopt/diff-files.h, gitopt/diff-index.h, gitopt/diff-tree.h, gitopt/diff.h, http-push.c, rev-list.c, revision.c, revision.h

diff --git a/builtin-diff.c b/builtin-diff.c
index d3ac581..3aefd3c 100644
--- a/builtin-diff.c
+++ b/builtin-diff.c
@@ -12,6 +12,9 @@ #include "diffcore.h"
 #include "revision.h"
 #include "log-tree.h"
 #include "builtin.h"
+#include "gitopt/diff-index.h"
+#include "gitopt/diff-files.h"
+#include "gitopt/diff-tree.h"
 
 /* NEEDSWORK: struct object has place for name but we _do_
  * know mode when we extracted the blob out of a tree, which
@@ -25,26 +28,25 @@ struct blobinfo {
 static const char builtin_diff_usage[] =
 "diff <options> <rev>{0,2} -- <path>*";
 
+static int extra_diff_flags[LAST_DIFF_EXTRA_ID] = { 0 };
+
 static int builtin_diff_files(struct rev_info *revs,
 			      int argc, const char **argv)
 {
-	int silent = 0;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--base"))
-			revs->max_count = 1;
-		else if (!strcmp(arg, "--ours"))
-			revs->max_count = 2;
-		else if (!strcmp(arg, "--theirs"))
-			revs->max_count = 3;
-		else if (!strcmp(arg, "-q"))
-			silent = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
+	int i;
+	struct df_extra_opts dfeo = { revs, 0 };
+
+	if (argc)
+		usage(builtin_diff_usage);
+
+	for (i = 1; i < ARRAY_SIZE(extra_diff_flags); i++) {
+		if (extra_diff_flags[i]) {
+			if (diff_files_opt_handler(NULL, i, &dfeo) > 0)
+				continue;
 			usage(builtin_diff_usage);
-		argv++; argc--;
+		}
 	}
+
 	/*
 	 * Make sure there are NO revision (i.e. pending object) parameter,
 	 * specified rev.max_count is reasonable (0 <= n <= 3), and
@@ -65,7 +67,7 @@ static int builtin_diff_files(struct rev
 	 */
 	if (revs->diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
 		revs->diffopt.output_format = DIFF_FORMAT_RAW;
-	return run_diff_files(revs, silent);
+	return run_diff_files(revs, dfeo.silent);
 }
 
 static void stuff_change(struct diff_options *opt,
@@ -107,14 +109,9 @@ static int builtin_diff_b_f(struct rev_i
 	/* Blob vs file in the working tree*/
 	struct stat st;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc)
+		usage(builtin_diff_usage);
+
 	if (lstat(path, &st))
 		die("'%s': %s", path, strerror(errno));
 	if (!(S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)))
@@ -135,14 +132,9 @@ static int builtin_diff_blobs(struct rev
 	/* Blobs */
 	unsigned mode = canon_mode(S_IFREG | 0644);
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc)
+		usage(builtin_diff_usage);
+
 	stuff_change(&revs->diffopt,
 		     mode, mode,
 		     blob[0].sha1, blob[1].sha1,
@@ -155,17 +147,19 @@ static int builtin_diff_blobs(struct rev
 static int builtin_diff_index(struct rev_info *revs,
 			      int argc, const char **argv)
 {
-	int cached = 0;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--cached"))
-			cached = 1;
-		else if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
+	int i, cached = 0;
+
+	if (argc)
+		usage(builtin_diff_usage);
+
+	for (i = 1; i < ARRAY_SIZE(extra_diff_flags); i++) {
+		if (extra_diff_flags[i]) {
+			if (diff_index_opt_handler(NULL, i, &cached) > 0)
+				continue;
 			usage(builtin_diff_usage);
-		argv++; argc--;
+		}
 	}
+
 	/*
 	 * Make sure there is one revision (i.e. pending object),
 	 * and there is no revision filtering parameters.
@@ -183,14 +177,9 @@ static int builtin_diff_tree(struct rev_
 {
 	const unsigned char *(sha1[2]);
 	int swap = 1;
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+
+	if (argc)
+		usage(builtin_diff_usage);
 
 	/* We saw two trees, ent[0] and ent[1].
 	 * unless ent[0] is unintesting, they are swapped
@@ -212,14 +201,9 @@ static int builtin_diff_combined(struct 
 	const unsigned char (*parent)[20];
 	int i;
 
-	while (1 < argc) {
-		const char *arg = argv[1];
-		if (!strcmp(arg, "--raw"))
-			revs->diffopt.output_format = DIFF_FORMAT_RAW;
-		else
-			usage(builtin_diff_usage);
-		argv++; argc--;
-	}
+	if (argc)
+		usage(builtin_diff_usage);
+
 	if (!revs->dense_combined_merges && !revs->combine_merges)
 		revs->dense_combined_merges = revs->combine_merges = 1;
 	parent = xmalloc(ents * sizeof(*parent));
@@ -243,6 +227,27 @@ static void add_head(struct rev_info *re
 	add_object(obj, &revs->pending_objects, NULL, "HEAD");
 }
 
+static struct opt_spec * diff_combined_ost()
+{
+	struct opt_spec *rv, *tmp;
+
+	tmp = combine_opt_spec(diff_files_ost, diff_tree_ost);
+	rv = combine_opt_spec(tmp, diff_index_ost);
+	free(tmp);
+	return rv;
+}
+
+/* just set flags in here for use by the builtin_diff_* functions  */
+static int diff_combined_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	if (id > 0 && id < LAST_DIFF_EXTRA_ID) {
+		extra_diff_flags[id] = 1;
+		return 1;
+	}
+	return 0;
+}
+
 int cmd_diff(int argc, const char **argv, char **envp)
 {
 	struct rev_info rev;
@@ -250,6 +255,9 @@ int cmd_diff(int argc, const char **argv
 	int ents = 0, blobs = 0, paths = 0;
 	const char *path = NULL;
 	struct blobinfo blob[2];
+	struct exec_args *b;
+	struct opt_spec *ost = diff_combined_ost();
+	struct gitopt_extra ge = { ost, diff_combined_handler, NULL };
 
 	/*
 	 * We could get N tree-ish in the rev.pending_objects list.
@@ -275,22 +283,14 @@ int cmd_diff(int argc, const char **argv
 	init_revisions(&rev);
 	rev.diffopt.output_format = DIFF_FORMAT_PATCH;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
+	b = setup_revisions(argc, argv, &rev, NULL, &ge);
+	free(ost);
+
 	/* Do we have --cached and not have a pending object, then
 	 * default to HEAD by hand.  Eek.
 	 */
-	if (!rev.pending_objects) {
-		int i;
-		for (i = 1; i < argc; i++) {
-			const char *arg = argv[i];
-			if (!strcmp(arg, "--"))
-				break;
-			else if (!strcmp(arg, "--cached")) {
-				add_head(&rev);
-				break;
-			}
-		}
-	}
+	if (!rev.pending_objects && extra_diff_flags[opt_cached])
+		add_head(&rev);
 
 	for (list = rev.pending_objects; list; list = list->next) {
 		struct object *obj = list->item;
@@ -340,17 +340,17 @@ int cmd_diff(int argc, const char **argv
 	if (!ents) {
 		switch (blobs) {
 		case 0:
-			return builtin_diff_files(&rev, argc, argv);
+			return builtin_diff_files(&rev, b->argc, b->argv);
 			break;
 		case 1:
 			if (paths != 1)
 				usage(builtin_diff_usage);
-			return builtin_diff_b_f(&rev, argc, argv, blob, path);
+			return builtin_diff_b_f(&rev, b->argc, b->argv, blob, path);
 			break;
 		case 2:
 			if (paths)
 				usage(builtin_diff_usage);
-			return builtin_diff_blobs(&rev, argc, argv, blob);
+			return builtin_diff_blobs(&rev, b->argc, b->argv, blob);
 			break;
 		default:
 			usage(builtin_diff_usage);
@@ -359,10 +359,10 @@ int cmd_diff(int argc, const char **argv
 	else if (blobs)
 		usage(builtin_diff_usage);
 	else if (ents == 1)
-		return builtin_diff_index(&rev, argc, argv);
+		return builtin_diff_index(&rev, b->argc, b->argv);
 	else if (ents == 2)
-		return builtin_diff_tree(&rev, argc, argv, ent);
+		return builtin_diff_tree(&rev, b->argc, b->argv, ent);
 	else
-		return builtin_diff_combined(&rev, argc, argv, ent, ents);
+		return builtin_diff_combined(&rev, b->argc, b->argv, ent, ents);
 	usage(builtin_diff_usage);
 }
diff --git a/builtin-log.c b/builtin-log.c
index 69f2911..3cfc8ac 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -14,14 +14,15 @@ static int cmd_log_wc(int argc, const ch
 		      struct rev_info *rev)
 {
 	struct commit *commit;
+	struct exec_args *b;
 
 	rev->abbrev = DEFAULT_ABBREV;
 	rev->commit_format = CMIT_FMT_DEFAULT;
 	rev->verbose_header = 1;
-	argc = setup_revisions(argc, argv, rev, "HEAD");
+	b = setup_revisions(argc, argv, rev, "HEAD", NULL);
 
-	if (argc > 1)
-		die("unrecognized argument: %s", argv[1]);
+	if (b->argc > 1)
+		die("unrecognized argument: %s", b->argv[0]);
 
 	prepare_revision_walk(rev);
 	setup_pager();
diff --git a/commit.c b/commit.c
index 2717dd8..2343729 100644
--- a/commit.c
+++ b/commit.c
@@ -24,19 +24,19 @@ const char *commit_type = "commit";
 
 enum cmit_fmt get_commit_format(const char *arg)
 {
-	if (!*arg)
+	if (!arg)
 		return CMIT_FMT_DEFAULT;
-	if (!strcmp(arg, "=raw"))
+	if (!strcmp(arg, "raw"))
 		return CMIT_FMT_RAW;
-	if (!strcmp(arg, "=medium"))
+	if (!strcmp(arg, "medium"))
 		return CMIT_FMT_MEDIUM;
-	if (!strcmp(arg, "=short"))
+	if (!strcmp(arg, "short"))
 		return CMIT_FMT_SHORT;
-	if (!strcmp(arg, "=full"))
+	if (!strcmp(arg, "full"))
 		return CMIT_FMT_FULL;
-	if (!strcmp(arg, "=fuller"))
+	if (!strcmp(arg, "fuller"))
 		return CMIT_FMT_FULLER;
-	if (!strcmp(arg, "=oneline"))
+	if (!strcmp(arg, "oneline"))
 		return CMIT_FMT_ONELINE;
 	die("invalid --pretty format");
 }
diff --git a/diff-files.c b/diff-files.c
index b9d193d..431487b 100644
--- a/diff-files.c
+++ b/diff-files.c
@@ -7,6 +7,7 @@ #include "cache.h"
 #include "diff.h"
 #include "commit.h"
 #include "revision.h"
+#include "gitopt/diff-files.h"
 
 static const char diff_files_usage[] =
 "git-diff-files [-q] [-0/-1/2/3 |-c|--cc] [<common diff options>] [<path>...]"
@@ -15,26 +16,18 @@ COMMON_DIFF_OPTIONS_HELP;
 int main(int argc, const char **argv)
 {
 	struct rev_info rev;
-	int silent = 0;
+	struct exec_args *b;
+	struct df_extra_opts dfeo = { &rev, 0 };
+	struct gitopt_extra ge = { diff_files_ost, diff_files_opt_handler,
+				   &dfeo };
 
 	git_config(git_diff_config);
 	init_revisions(&rev);
 	rev.abbrev = 0;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
-	while (1 < argc && argv[1][0] == '-') {
-		if (!strcmp(argv[1], "--base"))
-			rev.max_count = 1;
-		else if (!strcmp(argv[1], "--ours"))
-			rev.max_count = 2;
-		else if (!strcmp(argv[1], "--theirs"))
-			rev.max_count = 3;
-		else if (!strcmp(argv[1], "-q"))
-			silent = 1;
-		else
-			usage(diff_files_usage);
-		argv++; argc--;
-	}
+	b = setup_revisions(argc, argv, &rev, NULL, &ge);
+	if (b->argc)
+		usage(diff_files_usage);
 	/*
 	 * Make sure there are NO revision (i.e. pending object) parameter,
 	 * rev.max_count is reasonable (0 <= n <= 3),
@@ -50,5 +43,5 @@ int main(int argc, const char **argv)
 	 */
 	if (rev.diffopt.output_format == DIFF_FORMAT_NO_OUTPUT)
 		rev.diffopt.output_format = DIFF_FORMAT_RAW;
-	return run_diff_files(&rev, silent);
+	return run_diff_files(&rev, dfeo.silent);
 }
diff --git a/diff-index.c b/diff-index.c
index 8c9f601..987a00a 100644
--- a/diff-index.c
+++ b/diff-index.c
@@ -2,6 +2,7 @@ #include "cache.h"
 #include "diff.h"
 #include "commit.h"
 #include "revision.h"
+#include "gitopt/diff-index.h"
 
 static const char diff_cache_usage[] =
 "git-diff-index [-m] [--cached] "
@@ -12,21 +13,18 @@ int main(int argc, const char **argv)
 {
 	struct rev_info rev;
 	int cached = 0;
-	int i;
+	struct exec_args *b;
+	struct gitopt_extra ge = { diff_index_ost,
+				diff_index_opt_handler, &cached };
 
 	git_config(git_diff_config);
 	init_revisions(&rev);
 	rev.abbrev = 0;
 
-	argc = setup_revisions(argc, argv, &rev, NULL);
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
-			
-		if (!strcmp(arg, "--cached"))
-			cached = 1;
-		else
-			usage(diff_cache_usage);
-	}
+	b = setup_revisions(argc, argv, &rev, NULL, &ge);
+	if (b->argc)
+		usage(diff_cache_usage);
+
 	/*
 	 * Make sure there is one revision (i.e. pending object),
 	 * and there is no revision filtering parameters.
diff --git a/diff-stages.c b/diff-stages.c
index dcd20e7..05129c5 100644
--- a/diff-stages.c
+++ b/diff-stages.c
@@ -4,6 +4,13 @@
 
 #include "cache.h"
 #include "diff.h"
+#include "gitopt.h"
+
+enum diff_stages_ost { opt_r = 1 };
+static const struct opt_spec diff_stages_ost[] = {
+	{ 0,	'r',	0,	0,	opt_r },
+	{ 0, 0 }
+};
 
 static struct diff_options diff_options;
 
@@ -59,40 +66,47 @@ int main(int ac, const char **av)
 	int stage1, stage2;
 	const char *prefix = setup_git_directory();
 	const char **pathspec = NULL;
+	struct exec_args *b;
+	struct opt_spec *ost;
+	struct gitopt_iterator gi;
+	int i;
 
 	git_config(git_diff_config);
 	read_cache();
 	diff_setup(&diff_options);
-	while (1 < ac && av[1][0] == '-') {
-		const char *arg = av[1];
-		if (!strcmp(arg, "-r"))
-			; /* as usual */
-		else {
-			int diff_opt_cnt;
-			diff_opt_cnt = diff_opt_parse(&diff_options,
-						      av+1, ac-1);
-			if (diff_opt_cnt < 0)
-				usage(diff_stages_usage);
-			else if (diff_opt_cnt) {
-				av += diff_opt_cnt;
-				ac -= diff_opt_cnt;
-				continue;
-			}
+
+	ost = combine_opt_spec(diff_ost, diff_stages_ost);
+	gitopt_iter_setup(&gi, ac, av);
+
+	for (b=gi.b; (i=gitopt_iter_parse(&gi, ost)); gitopt_iter_next(&gi)) {
+		switch (i) {
+		case GITOPT_NON_OPTION:
+			b->argv[b->argc++] = gi.argv[gi.pos];
+		case GITOPT_DD:
+		case opt_r: /* as usual */
+			break;
+		case GITOPT_ERROR:
+			usage(diff_stages_usage);
+		default:
+			i = diff_opt_handler(&gi, i, &diff_options);
+			if (i > 0)
+				gi.pos += i - 1;
 			else
 				usage(diff_stages_usage);
 		}
-		ac--; av++;
 	}
+	gitopt_iter_done(&gi);
+	free(ost);
 
-	if (ac < 3 ||
-	    sscanf(av[1], "%d", &stage1) != 1 ||
+	if (b->argc < 2 ||
+	    sscanf(b->argv[0], "%d", &stage1) != 1 ||
 	    ! (0 <= stage1 && stage1 <= 3) ||
-	    sscanf(av[2], "%d", &stage2) != 1 ||
+	    sscanf(b->argv[1], "%d", &stage2) != 1 ||
 	    ! (0 <= stage2 && stage2 <= 3))
 		usage(diff_stages_usage);
 
-	av += 3; /* The rest from av[0] are for paths restriction. */
-	pathspec = get_pathspec(prefix, av);
+	b->argv += 2; /* The rest from b->argv[0] are for paths restriction. */
+	pathspec = get_pathspec(prefix, b->argv);
 
 	if (diff_setup_done(&diff_options) < 0)
 		usage(diff_stages_usage);
diff --git a/diff-tree.c b/diff-tree.c
index 7207867..b5c6072 100644
--- a/diff-tree.c
+++ b/diff-tree.c
@@ -2,6 +2,7 @@ #include "cache.h"
 #include "diff.h"
 #include "commit.h"
 #include "log-tree.h"
+#include "gitopt/diff-tree.h"
 
 static struct rev_info log_tree_opt;
 
@@ -66,23 +67,19 @@ int main(int argc, const char **argv)
 	static struct rev_info *opt = &log_tree_opt;
 	struct object_list *list;
 	int read_stdin = 0;
+	struct exec_args *b;
+	struct gitopt_extra ge = { diff_tree_ost, diff_tree_opt_handler,
+				&read_stdin };
 
 	git_config(git_diff_config);
 	nr_sha1 = 0;
 	init_revisions(opt);
 	opt->abbrev = 0;
 	opt->diff = 1;
-	argc = setup_revisions(argc, argv, opt, NULL);
+	b = setup_revisions(argc, argv, opt, NULL, &ge);
 
-	while (--argc > 0) {
-		const char *arg = *++argv;
-
-		if (!strcmp(arg, "--stdin")) {
-			read_stdin = 1;
-			continue;
-		}
+	if (b->argc)
 		usage(diff_tree_usage);
-	}
 
 	/*
 	 * NOTE! "setup_revisions()" will have inserted the revisions
diff --git a/diff.c b/diff.c
index 7a7b839..7d88dc5 100644
--- a/diff.c
+++ b/diff.c
@@ -10,6 +10,48 @@ #include "diff.h"
 #include "diffcore.h"
 #include "delta.h"
 #include "xdiff-interface.h"
+#include "gitopt.h"
+
+static int diff_scoreopt_parse(const int id, const char *opt);
+
+enum diff_ost_ids {
+	opt_p = GITOPT_DIFF_BASE, opt_raw, opt_patch_with_raw,
+	opt_stat, opt_patch_with_stat,
+	opt_z, opt_l, opt_full_index, opt_name_only, opt_name_status,
+	opt_R, opt_S, opt_s, opt_O, opt_diff_filter,
+	opt_pickaxe_all, opt_pickaxe_regex,
+	opt_B, opt_M, opt_C, opt_find_copies_harder, opt_abbrev,
+	opt_binary
+};
+
+const struct opt_spec diff_ost[] = {
+	{ 0,			'p',	0,	0,	opt_p },
+	{ "unified",		'u',	0,	0,	opt_p },
+	{ "raw",		0,	0,	0,	opt_raw },
+	{ "patch-with-raw",	0,	0,	0,	opt_patch_with_raw },
+	{ "stat",		0,	0,	0,	opt_stat },
+	{ "patch-with-stat",	0,	0,	0,	opt_patch_with_stat },
+	{ 0,			'z',	0,	0,	opt_z },
+	{ 0,			'l',	0,	ARG_INT,	opt_l },
+	{ "full-index",		0,	0,	0,	opt_full_index },
+	{ "name-only",		0,	0,	0,	opt_name_only },
+	{ "name-status",	0,	0,	0,	opt_name_status },
+	{ 0,			'R',	0,	0,	opt_R },
+	{ 0,			'S',	0,	ARG_ONE,	opt_S },
+	{ 0,			's',	0,	0,	opt_s },
+	{ 0,			'O',	0,	ARG_ONE, opt_O },
+	{ "diff-filter",	0,	0,	ARG_ONE, opt_diff_filter },
+	{ "pickaxe-all",	0,	0,	0,	opt_pickaxe_all },
+	{ "pickaxe-regex",	0,	0,	0,	opt_pickaxe_regex },
+	{ 0,			'B',	0,	ARG_OPT,	opt_B },
+	{ 0,			'M',	0,	ARG_OPT,	opt_M },
+	{ 0,			'C',	0,	ARG_OPT,	opt_C },
+	{ "find-copies-harder",	0,	0,	0,	opt_find_copies_harder},
+	{ "abbrev",		0,	0,	ARG_OPTINT, opt_abbrev },
+	{ "binary",		0,	0,	0,	opt_binary },
+	{ 0, 0 }
+};
+
 
 static int use_size_cache;
 
@@ -1222,79 +1264,102 @@ int diff_setup_done(struct diff_options 
 	return 0;
 }
 
-int diff_opt_parse(struct diff_options *options, const char **av, int ac)
+int diff_opt_handler(struct gitopt_iterator *gi, const int id, void *args)
 {
-	const char *arg = av[0];
-	if (!strcmp(arg, "-p") || !strcmp(arg, "-u"))
+	struct diff_options *options = (struct diff_options *)args;
+
+	switch (id) {
+	case opt_p:
 		options->output_format = DIFF_FORMAT_PATCH;
-	else if (!strcmp(arg, "--patch-with-raw")) {
+		break;
+	case opt_raw:
+		options->output_format = DIFF_FORMAT_RAW;
+		break;
+	case opt_patch_with_raw:
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->with_raw = 1;
-	}
-	else if (!strcmp(arg, "--stat"))
+		break;
+	case opt_stat:
 		options->output_format = DIFF_FORMAT_DIFFSTAT;
-	else if (!strcmp(arg, "--patch-with-stat")) {
+		break;
+	case opt_patch_with_stat:
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->with_stat = 1;
-	}
-	else if (!strcmp(arg, "-z"))
+		break;
+	case opt_z:
 		options->line_termination = 0;
-	else if (!strncmp(arg, "-l", 2))
-		options->rename_limit = strtoul(arg+2, NULL, 10);
-	else if (!strcmp(arg, "--full-index"))
+		break;
+	case opt_l:
+		options->rename_limit = strtoul(gi->ea->argv[0], NULL, 10);
+		break;
+	case opt_full_index:
 		options->full_index = 1;
-	else if (!strcmp(arg, "--binary")) {
+		break;
+	case opt_binary:
 		options->output_format = DIFF_FORMAT_PATCH;
 		options->full_index = options->binary = 1;
-	}
-	else if (!strcmp(arg, "--name-only"))
+		break;
+	case opt_name_only:
 		options->output_format = DIFF_FORMAT_NAME;
-	else if (!strcmp(arg, "--name-status"))
+		break;
+	case opt_name_status:
 		options->output_format = DIFF_FORMAT_NAME_STATUS;
-	else if (!strcmp(arg, "-R"))
+		break;
+	case opt_R:
 		options->reverse_diff = 1;
-	else if (!strncmp(arg, "-S", 2))
-		options->pickaxe = arg + 2;
-	else if (!strcmp(arg, "-s"))
+		break;
+	case opt_S:
+		options->pickaxe = gi->ea->argv[0];
+		break;
+	case opt_s:
 		options->output_format = DIFF_FORMAT_NO_OUTPUT;
-	else if (!strncmp(arg, "-O", 2))
-		options->orderfile = arg + 2;
-	else if (!strncmp(arg, "--diff-filter=", 14))
-		options->filter = arg + 14;
-	else if (!strcmp(arg, "--pickaxe-all"))
+		break;
+	case opt_O:
+		options->orderfile = gi->ea->argv[0];
+		break;
+	case opt_diff_filter:
+		options->filter = gi->ea->argv[0];
+		break;
+	case opt_pickaxe_all:
 		options->pickaxe_opts = DIFF_PICKAXE_ALL;
-	else if (!strcmp(arg, "--pickaxe-regex"))
+		break;
+	case opt_pickaxe_regex:
 		options->pickaxe_opts = DIFF_PICKAXE_REGEX;
-	else if (!strncmp(arg, "-B", 2)) {
-		if ((options->break_opt =
-		     diff_scoreopt_parse(arg)) == -1)
+		break;
+	case opt_B:
+		if (gi->ea->argc && (options->break_opt =
+		     diff_scoreopt_parse(id, gi->ea->argv[1])) == -1)
 			return -1;
-	}
-	else if (!strncmp(arg, "-M", 2)) {
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+		break;
+	case opt_M:
+		if (gi->ea->argc && (options->rename_score =
+		     diff_scoreopt_parse(id, gi->ea->argv[1])) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_RENAME;
-	}
-	else if (!strncmp(arg, "-C", 2)) {
-		if ((options->rename_score =
-		     diff_scoreopt_parse(arg)) == -1)
+		break;
+	case opt_C:
+		if (gi->ea->argc && (options->rename_score =
+		     diff_scoreopt_parse(id, gi->ea->argv[1])) == -1)
 			return -1;
 		options->detect_rename = DIFF_DETECT_COPY;
-	}
-	else if (!strcmp(arg, "--find-copies-harder"))
+		break;
+	case opt_find_copies_harder:
 		options->find_copies_harder = 1;
-	else if (!strcmp(arg, "--abbrev"))
-		options->abbrev = DEFAULT_ABBREV;
-	else if (!strncmp(arg, "--abbrev=", 9)) {
-		options->abbrev = strtoul(arg + 9, NULL, 10);
-		if (options->abbrev < MINIMUM_ABBREV)
-			options->abbrev = MINIMUM_ABBREV;
-		else if (40 < options->abbrev)
-			options->abbrev = 40;
-	}
-	else
+		break;
+	case opt_abbrev:
+		if (gi->ea->argc == 1)
+			options->abbrev = DEFAULT_ABBREV;
+		else {
+			options->abbrev = strtoul(gi->ea->argv[1], NULL, 10);
+			if (options->abbrev < MINIMUM_ABBREV)
+				options->abbrev = MINIMUM_ABBREV;
+			else if (40 < options->abbrev)
+				options->abbrev = 40;
+		}
+		break;
+	default:
 		return 0;
+	}
 	return 1;
 }
 
@@ -1304,9 +1369,13 @@ static int parse_num(const char **cp_p)
 	int ch, dot;
 	const char *cp = *cp_p;
 
+	if (!cp)
+		return 0;
+
 	num = 0;
 	scale = 1;
 	dot = 0;
+
 	for(;;) {
 		ch = *cp;
 		if ( !dot && ch == '.' ) {
@@ -1334,21 +1403,15 @@ static int parse_num(const char **cp_p)
 	return (num >= scale) ? MAX_SCORE : (MAX_SCORE * num / scale);
 }
 
-int diff_scoreopt_parse(const char *opt)
+static int diff_scoreopt_parse(const int id, const char *opt)
 {
-	int opt1, opt2, cmd;
-
-	if (*opt++ != '-')
-		return -1;
-	cmd = *opt++;
-	if (cmd != 'M' && cmd != 'C' && cmd != 'B')
-		return -1; /* that is not a -M, -C nor -B option */
+	int opt1, opt2;
 
 	opt1 = parse_num(&opt);
-	if (cmd != 'B')
+	if (id != opt_B)
 		opt2 = 0;
 	else {
-		if (*opt == 0)
+		if (!opt || *opt == 0)
 			opt2 = 0;
 		else if (*opt != '/')
 			return -1; /* we expect -B80/99 or -B80 */
@@ -1357,7 +1420,7 @@ int diff_scoreopt_parse(const char *opt)
 			opt2 = parse_num(&opt);
 		}
 	}
-	if (*opt != 0)
+	if (opt && *opt != 0)
 		return -1;
 	return opt1 | (opt2 << 16);
 }
diff --git a/diff.h b/diff.h
index d052608..fa44d1b 100644
--- a/diff.h
+++ b/diff.h
@@ -5,6 +5,7 @@ #ifndef DIFF_H
 #define DIFF_H
 
 #include "tree-walk.h"
+#include "gitopt.h"
 
 struct rev_info;
 struct diff_options;
@@ -96,15 +97,13 @@ extern void diff_change(struct diff_opti
 extern void diff_unmerge(struct diff_options *,
 			 const char *path);
 
-extern int diff_scoreopt_parse(const char *opt);
-
 #define DIFF_SETUP_REVERSE      	1
 #define DIFF_SETUP_USE_CACHE		2
 #define DIFF_SETUP_USE_SIZE_CACHE	4
 
 extern int git_diff_config(const char *var, const char *value);
 extern void diff_setup(struct diff_options *);
-extern int diff_opt_parse(struct diff_options *, const char **, int);
+extern int diff_opt_handler(struct gitopt_iterator *, const int, void *);
 extern int diff_setup_done(struct diff_options *);
 
 #define DIFF_DETECT_RENAME	1
@@ -117,6 +116,8 @@ extern void diffcore_std(struct diff_opt
 
 extern void diffcore_std_no_resolve(struct diff_options *);
 
+extern const struct opt_spec diff_ost[];
+
 #define COMMON_DIFF_OPTIONS_HELP \
 "\ncommon diff options:\n" \
 "  -z            output diff-raw with lines terminated with NUL.\n" \
diff --git a/gitopt/diff-files.h b/gitopt/diff-files.h
new file mode 100644
index 0000000..85c84ba
--- /dev/null
+++ b/gitopt/diff-files.h
@@ -0,0 +1,45 @@
+#ifndef GITOPT_DIFF_FILES_H
+#define GITOPT_DIFF_FILES_H
+
+#include "../gitopt/diff.h"
+#include "../revision.h"
+
+struct df_extra_opts {
+	struct rev_info *rev;
+	int silent;
+};
+
+static const struct opt_spec diff_files_ost[] = {
+	{ "base",	0,	0,	0,	opt_base },
+	{ "ours",	0,	0,	0,	opt_ours },
+	{ "theirs",	0,	0,	0,	opt_theirs },
+	{ 0,		'q',	0,	0,	opt_q },
+	{ 0, 0 }
+};
+
+static int diff_files_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	int rv = 1;
+	struct df_extra_opts *dfeo = (struct df_extra_opts *)args;
+
+	switch (id) {
+	case opt_base:
+		dfeo->rev->max_count = 1;
+		break;
+	case opt_ours:
+		dfeo->rev->max_count = 2;
+		break;
+	case opt_theirs:
+		dfeo->rev->max_count = 3;
+		break;
+	case opt_q:
+		dfeo->silent = 1;
+		break;
+	default:
+		rv = 0;
+	}
+	return rv;
+}
+
+#endif
diff --git a/gitopt/diff-index.h b/gitopt/diff-index.h
new file mode 100644
index 0000000..aff0944
--- /dev/null
+++ b/gitopt/diff-index.h
@@ -0,0 +1,22 @@
+#ifndef GITOPT_DIFF_INDEX_H
+#define GITOPT_DIFF_INDEX_H
+
+#include "../gitopt/diff.h"
+
+static const struct opt_spec diff_index_ost[] = {
+	{ "cached",	0,	0,	0,	opt_cached },
+	{ 0, 0 }
+};
+
+static int diff_index_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	if (id == opt_cached) {
+		*(int *)args = 1;
+		return 1;
+	}
+	return 0;
+}
+
+#endif
+
diff --git a/gitopt/diff-tree.h b/gitopt/diff-tree.h
new file mode 100644
index 0000000..cfee9b5
--- /dev/null
+++ b/gitopt/diff-tree.h
@@ -0,0 +1,22 @@
+#ifndef GITOPT_DIFF_TREE_H
+#define GITOPT_DIFF_TREE_H
+
+#include "../gitopt/diff.h"
+
+static const struct opt_spec diff_tree_ost[] = {
+	{ "stdin",	0,	0,	0,	opt_stdin },
+	{ 0, 0 }
+};
+
+/* inline to suppress a warning as it's not used in builtin_diff_tree */
+static inline int diff_tree_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	if (id == opt_stdin) {
+		*(int *)args = 1;
+		return 1;
+	}
+	return 0;
+}
+
+#endif
diff --git a/gitopt/diff.h b/gitopt/diff.h
new file mode 100644
index 0000000..abc2c7a
--- /dev/null
+++ b/gitopt/diff.h
@@ -0,0 +1,19 @@
+#ifndef GITOPT_DIFF_H
+#define GITOPT_DIFF_H
+
+#include "../gitopt.h"
+
+enum diff_extra_ids {
+	/* diff-files */
+	opt_base = 1, opt_ours, opt_theirs, opt_q,
+
+	/* diff-tree */
+	opt_stdin,
+
+	/* diff-index */
+	opt_cached,
+
+	LAST_DIFF_EXTRA_ID,
+};
+
+#endif /* GITOPT_DIFF_H */
diff --git a/http-push.c b/http-push.c
index b4327d9..9c16f3b 100644
--- a/http-push.c
+++ b/http-push.c
@@ -2499,7 +2499,7 @@ int main(int argc, char **argv)
 			commit_argc++;
 		}
 		init_revisions(&revs);
-		setup_revisions(commit_argc, commit_argv, &revs, NULL);
+		setup_revisions(commit_argc, commit_argv, &revs, NULL, NULL);
 		free(new_sha1_hex);
 		if (old_sha1_hex) {
 			free(old_sha1_hex);
diff --git a/rev-list.c b/rev-list.c
index 8b0ec38..7baeb52 100644
--- a/rev-list.c
+++ b/rev-list.c
@@ -291,34 +291,49 @@ static void mark_edges_uninteresting(str
 	}
 }
 
+enum rev_list_ids { opt_header = 1, opt_timestamp, opt_bisect };
+
+static const struct opt_spec rev_list_ost[] = {
+	{ "header",	0,	0,	0,	opt_header },
+	{ "timestamp",	0,	0,	0,	opt_timestamp },
+	{ "bisect",	0,	0,	0,	opt_bisect },
+	{ 0, 0 }
+};
+
+static int rev_list_opt_handler(struct gitopt_iterator *gi,
+				const int id, void *args)
+{
+	int rv = 1;
+	switch (id) {
+	case opt_header:
+		revs.verbose_header = 1;
+		break;
+	case opt_timestamp:
+		show_timestamp = 1;
+		break;
+	case opt_bisect:
+		bisect_list = 1;
+		break;
+	default:
+		rv = 0;
+	}
+	return rv;
+}
+
 int main(int argc, const char **argv)
 {
 	struct commit_list *list;
-	int i;
+	struct exec_args *b;
+	struct gitopt_extra ge = { rev_list_ost, rev_list_opt_handler, NULL };
 
 	init_revisions(&revs);
 	revs.abbrev = 0;
 	revs.commit_format = CMIT_FMT_UNSPECIFIED;
-	argc = setup_revisions(argc, argv, &revs, NULL);
-
-	for (i = 1 ; i < argc; i++) {
-		const char *arg = argv[i];
+	b = setup_revisions(argc, argv, &revs, NULL, &ge);
 
-		if (!strcmp(arg, "--header")) {
-			revs.verbose_header = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--timestamp")) {
-			show_timestamp = 1;
-			continue;
-		}
-		if (!strcmp(arg, "--bisect")) {
-			bisect_list = 1;
-			continue;
-		}
+	if (b->argc)
 		usage(rev_list_usage);
 
-	}
 	if (revs.commit_format != CMIT_FMT_UNSPECIFIED) {
 		/* The command line has a --pretty  */
 		hdr_termination = '\n';
diff --git a/revision.c b/revision.c
index 2294b16..aeaf52c 100644
--- a/revision.c
+++ b/revision.c
@@ -6,6 +6,7 @@ #include "commit.h"
 #include "diff.h"
 #include "refs.h"
 #include "revision.h"
+#include "gitopt.h"
 
 static char *path_name(struct name_path *path, const char *name)
 {
@@ -534,6 +535,136 @@ void init_revisions(struct rev_info *rev
 	diff_setup(&revs->diffopt);
 }
 
+enum setup_revision_ost_ids {
+	opt_max_count = GITOPT_SR_BASE, opt_max_age, opt_min_age,
+	opt_since, opt_until, opt_all, opt_not, opt_default,
+	opt_topo_order, opt_date_order, opt_parents, opt_dense,
+	opt_sparse, opt_remove_empty, opt_no_merges, opt_boundary,
+	opt_objects, opt_objects_edge, opt_unpacked,
+	opt_r, opt_t, opt_m, opt_c, opt_cc, opt_v,
+	opt_pretty, opt_root, opt_no_commit_id, opt_always,
+	opt_abbrev, opt_no_abbrev, opt_abbrev_commit, opt_full_diff
+};
+
+static const struct opt_spec setup_revisions_ost[] = {
+	{ "max-count",		'n',	0,	ARG_INT,	opt_max_count },
+	{ 0,			' ',	0,	ARG_INT,	opt_max_count },
+	{ "max-age",		0,	0,	ARG_INT,	opt_max_age },
+	{ "min-age",		0,	0,	ARG_INT,	opt_min_age },
+	{ "since",		0,	0,	ARG_ONE,	opt_since },
+	{ "after",		0,	0,	ARG_ONE,	opt_since },
+	{ "before",		0,	0,	ARG_ONE,	opt_until },
+	{ "until",		0,	0,	ARG_ONE,	opt_until },
+	{ "all",		0,	0,	0,		opt_all },
+	{ "not",		0,	0,	0,		opt_not },
+	{ "default",		0,	0,	ARG_ONE,	opt_default },
+	{ "topo-order",		0,	0,	0,	opt_topo_order },
+	{ "date-order",		0,	0,	0,	opt_date_order },
+	{ "parents",		0,	0,	0,	opt_parents },
+	{ "dense",		0,	0,	0,	opt_dense },
+	{ "sparse",		0,	0,	0,	opt_sparse },
+	{ "remove-empty",	0,	0,	0,	opt_remove_empty },
+	{ "no-merges",		0,	0,	0,	opt_no_merges },
+	{ "boundary",		0,	0,	0,	opt_boundary },
+	{ "objects",		0,	0,	0,	opt_objects },
+	{ "objects-edge",	0,	0,	0,	opt_objects_edge },
+	{ "unpacked",		0,	0,	0,	opt_unpacked },
+	{ 0,			'r',	0,	0,	opt_r },
+	{ 0,			't',	0,	0,	opt_t },
+	{ 0,			'm',	0,	0,	opt_m },
+	{ 0,			'c',	0,	0,	opt_c },
+	{ "cc",			0,	0,	0,	opt_cc },
+	{ 0,			'v',	0,	0,	opt_v },
+	{ "pretty",		0,	0,	ARG_OPT,	opt_pretty },
+	{ "root",		0,	0,	0,	opt_root },
+	{ "no-commit-id",	0,	0,	0,	opt_no_commit_id },
+	{ "always",		0,	0,	0,	opt_always },
+	{ "no-abbrev",		0,	0,	0,	opt_no_abbrev },
+	{ "abbrev",		0,	0,	0,	opt_abbrev },
+	{ "abbrev-commit",	0,	0,	0,	opt_abbrev_commit },
+	{ "full-diff",		0,	0,	0,	opt_full_diff },
+	{ 0, 0 }
+};
+
+static int setup_revision(struct gitopt_iterator *gi, struct rev_info *revs,
+			int flags, int seen_dashdash)
+{
+	const char *arg = gi->argv[gi->pos];
+	unsigned char sha1[20];
+	struct object *object;
+	char *dotdot;
+	int local_flags;
+
+	dotdot = strstr(arg, "..");
+	if (dotdot) {
+		unsigned char from_sha1[20];
+		const char *next = dotdot + 2;
+		const char *this = arg;
+		*dotdot = 0;
+		if (!*next)
+			next = "HEAD";
+		if (dotdot == arg)
+			this = "HEAD";
+		if (!get_sha1(this, from_sha1) &&
+		    !get_sha1(next, sha1)) {
+			struct object *exclude;
+			struct object *include;
+
+			exclude = get_reference(revs, this, from_sha1,
+						flags ^ UNINTERESTING);
+			include = get_reference(revs, next, sha1, flags);
+			if (!exclude || !include)
+				die("Invalid revision range %s..%s", arg, next);
+
+			if (!seen_dashdash) {
+				*dotdot = '.';
+				verify_non_filename(revs->prefix, arg);
+			}
+			add_pending_object(revs, exclude, this);
+			add_pending_object(revs, include, next);
+			return 1;
+		}
+		*dotdot = '.';
+	}
+
+	dotdot = strstr(arg, "^@");
+	if (dotdot && !dotdot[2]) {
+		*dotdot = 0;
+		if (add_parents_only(revs, arg, flags))
+			return 1;
+		*dotdot = '^';
+	}
+	local_flags = 0;
+	if (*arg == '^') {
+		local_flags = UNINTERESTING;
+		arg++;
+	}
+	if (get_sha1(arg, sha1) < 0) {
+		int j;
+
+		if (seen_dashdash || local_flags)
+			die("bad revision '%s'", arg);
+
+		/* If we didn't have a "--":
+		 * (1) all filenames must exist;
+		 * (2) all rev-args must not be interpretable
+		 *     as a valid filename.
+		 * but the latter we have checked in the main loop.
+		 */
+		for (j = gi->pos; j < gi->argc; j++)
+			verify_filename(revs->prefix, gi->argv[j]);
+
+		revs->prune_data = get_pathspec(revs->prefix,
+						gi->argv + gi->pos);
+		return 1;
+	}
+	if (!seen_dashdash)
+		verify_non_filename(revs->prefix, arg);
+	object = get_reference(revs, arg, sha1, flags ^ local_flags);
+	add_pending_object(revs, object, arg);
+	return 1;
+}
+
 /*
  * Parse revision information, filling in the "rev_info" structure,
  * and removing the used arguments from the argument list.
@@ -541,11 +672,14 @@ void init_revisions(struct rev_info *rev
  * Returns the number of arguments left that weren't recognized
  * (which are also moved to the head of the argument list)
  */
-int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def)
+struct exec_args *setup_revisions(int argc, const char **argv,
+					struct rev_info *revs, const char *def,
+					const struct gitopt_extra *ge)
 {
-	int i, flags, seen_dashdash;
-	const char **unrecognized = argv + 1;
-	int left = 1;
+	int i, id, flags, seen_dashdash;
+	struct gitopt_iterator gi;
+	struct exec_args *ea, *b;
+	struct opt_spec *ost;
 
 	/* First, search for "--" */
 	seen_dashdash = 0;
@@ -561,261 +695,158 @@ int setup_revisions(int argc, const char
 	}
 
 	flags = 0;
-	for (i = 1; i < argc; i++) {
-		struct object *object;
-		const char *arg = argv[i];
-		unsigned char sha1[20];
-		char *dotdot;
-		int local_flags;
+	ost = combine_opt_spec(setup_revisions_ost, diff_ost);
+	if (ge) {
+		struct opt_spec *tmp = ost;
+		ost = combine_opt_spec(tmp, ge->ost);
+		free(tmp);
+	}
 
-		if (*arg == '-') {
-			int opts;
-			if (!strncmp(arg, "--max-count=", 12)) {
-				revs->max_count = atoi(arg + 12);
-				continue;
-			}
-			/* accept -<digit>, like traditional "head" */
-			if ((*arg == '-') && isdigit(arg[1])) {
-				revs->max_count = atoi(arg + 1);
-				continue;
-			}
-			if (!strcmp(arg, "-n")) {
-				if (argc <= i + 1)
-					die("-n requires an argument");
-				revs->max_count = atoi(argv[++i]);
-				continue;
-			}
-			if (!strncmp(arg,"-n",2)) {
-				revs->max_count = atoi(arg + 2);
-				continue;
-			}
-			if (!strncmp(arg, "--max-age=", 10)) {
-				revs->max_age = atoi(arg + 10);
-				continue;
-			}
-			if (!strncmp(arg, "--since=", 8)) {
-				revs->max_age = approxidate(arg + 8);
-				continue;
-			}
-			if (!strncmp(arg, "--after=", 8)) {
-				revs->max_age = approxidate(arg + 8);
-				continue;
-			}
-			if (!strncmp(arg, "--min-age=", 10)) {
-				revs->min_age = atoi(arg + 10);
-				continue;
-			}
-			if (!strncmp(arg, "--before=", 9)) {
-				revs->min_age = approxidate(arg + 9);
-				continue;
-			}
-			if (!strncmp(arg, "--until=", 8)) {
-				revs->min_age = approxidate(arg + 8);
-				continue;
-			}
-			if (!strcmp(arg, "--all")) {
-				handle_all(revs, flags);
-				continue;
-			}
-			if (!strcmp(arg, "--not")) {
-				flags ^= UNINTERESTING;
-				continue;
-			}
-			if (!strcmp(arg, "--default")) {
-				if (++i >= argc)
-					die("bad --default argument");
-				def = argv[i];
-				continue;
-			}
-			if (!strcmp(arg, "--topo-order")) {
-				revs->topo_order = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--date-order")) {
-				revs->lifo = 0;
-				revs->topo_order = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--parents")) {
-				revs->parents = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--dense")) {
-				revs->dense = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--sparse")) {
-				revs->dense = 0;
-				continue;
-			}
-			if (!strcmp(arg, "--remove-empty")) {
-				revs->remove_empty_trees = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--no-merges")) {
-				revs->no_merges = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--boundary")) {
-				revs->boundary = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--objects")) {
-				revs->tag_objects = 1;
-				revs->tree_objects = 1;
-				revs->blob_objects = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--objects-edge")) {
-				revs->tag_objects = 1;
-				revs->tree_objects = 1;
-				revs->blob_objects = 1;
-				revs->edge_hint = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--unpacked")) {
-				revs->unpacked = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-r")) {
-				revs->diff = 1;
-				revs->diffopt.recursive = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-t")) {
-				revs->diff = 1;
-				revs->diffopt.recursive = 1;
-				revs->diffopt.tree_in_recursive = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-m")) {
-				revs->ignore_merges = 0;
-				continue;
-			}
-			if (!strcmp(arg, "-c")) {
-				revs->diff = 1;
-				revs->dense_combined_merges = 0;
-				revs->combine_merges = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--cc")) {
-				revs->diff = 1;
-				revs->dense_combined_merges = 1;
-				revs->combine_merges = 1;
-				continue;
-			}
-			if (!strcmp(arg, "-v")) {
-				revs->verbose_header = 1;
-				continue;
-			}
-			if (!strncmp(arg, "--pretty", 8)) {
-				revs->verbose_header = 1;
-				revs->commit_format = get_commit_format(arg+8);
-				continue;
-			}
-			if (!strcmp(arg, "--root")) {
-				revs->show_root_diff = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--no-commit-id")) {
-				revs->no_commit_id = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--always")) {
-				revs->always_show_header = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--no-abbrev")) {
-				revs->abbrev = 0;
-				continue;
-			}
-			if (!strcmp(arg, "--abbrev")) {
+	gitopt_iter_setup(&gi, argc, argv);
+	ea = gi.ea;
+	b = gi.b;
+	for (; (id = gitopt_iter_parse(&gi, ost)); gitopt_iter_next(&gi)) {
+		switch (id) {
+		case opt_max_count:
+			revs->max_count = atoi(ea->argv[0]);
+			break;
+		case opt_max_age:
+			revs->max_age = atoi(ea->argv[0]);
+			break;
+		case opt_min_age:
+			revs->min_age = atoi(ea->argv[0]);
+			break;
+		case opt_since:
+			revs->max_age = approxidate(ea->argv[0]);
+			break;
+		case opt_until:
+			revs->min_age = approxidate(ea->argv[0]);
+			break;
+		case opt_all:
+			handle_all(revs, flags);
+			break;
+		case opt_not:
+			flags ^= UNINTERESTING;
+			break;
+		case opt_default:
+			if (!ea->argc)
+				die("bad --default argument");
+			def = ea->argv[0];
+			break;
+		case opt_topo_order:
+			revs->topo_order = 1;
+			break;
+		case opt_date_order:
+			revs->lifo = 0;
+			revs->topo_order = 1;
+			break;
+		case opt_parents:
+			revs->parents = 1;
+			break;
+		case opt_dense:
+			revs->dense = 1;
+			break;
+		case opt_sparse:
+			revs->dense = 0;
+			break;
+		case opt_remove_empty:
+			revs->remove_empty_trees = 1;
+			break;
+		case opt_no_merges:
+			revs->no_merges = 1;
+			break;
+		case opt_boundary:
+			revs->boundary = 1;
+			break;
+		case opt_objects_edge:
+			revs->edge_hint = 1; /* fall through */
+		case opt_objects:
+			revs->tag_objects = 1;
+			revs->tree_objects = 1;
+			revs->blob_objects = 1;
+			break;
+		case opt_unpacked:
+			revs->unpacked = 1;
+			break;
+		case opt_r:
+			revs->diff = 1;
+			revs->diffopt.recursive = 1;
+			break;
+		case opt_t:
+			revs->diff = 1;
+			revs->diffopt.recursive = 1;
+			revs->diffopt.tree_in_recursive = 1;
+			break;
+		case opt_m:
+			revs->ignore_merges = 0;
+			break;
+		case opt_c:
+			revs->diff = 1;
+			revs->dense_combined_merges = 0;
+			revs->combine_merges = 1;
+			break;
+		case opt_cc:
+			revs->diff = 1;
+			revs->dense_combined_merges = 1;
+			revs->combine_merges = 1;
+			break;
+		case opt_v:
+			revs->verbose_header = 1;
+			break;
+		case opt_pretty:
+			revs->verbose_header = 1;
+			revs->commit_format = get_commit_format(ea->argv[1]);
+			break;
+		case opt_root:
+			revs->show_root_diff = 1;
+			break;
+		case opt_no_commit_id:
+			revs->no_commit_id = 1;
+			break;
+		case opt_always:
+			revs->always_show_header = 1;
+			break;
+		case opt_no_abbrev:
+			revs->abbrev = 0;
+			break;
+		case opt_abbrev_commit:
+			revs->abbrev_commit = 1;
+			break;
+		case opt_full_diff:
+			revs->diff = 1;
+			revs->full_diff = 1;
+			break;
+		case opt_abbrev:
+			if (ea->argc == 1)
 				revs->abbrev = DEFAULT_ABBREV;
-				continue;
+			else {
+				revs->abbrev = strtoul(ea->argv[1], NULL, 10);
+				if (revs->abbrev < MINIMUM_ABBREV)
+					revs->abbrev = MINIMUM_ABBREV;
+				else if (40 < revs->abbrev)
+					revs->abbrev = 40;
 			}
-			if (!strcmp(arg, "--abbrev-commit")) {
-				revs->abbrev_commit = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--full-diff")) {
-				revs->diff = 1;
-				revs->full_diff = 1;
-				continue;
-			}
-			opts = diff_opt_parse(&revs->diffopt, argv+i, argc-i);
-			if (opts > 0) {
-				revs->diff = 1;
-				i += opts - 1;
-				continue;
-			}
-			*unrecognized++ = arg;
-			left++;
-			continue;
-		}
-		dotdot = strstr(arg, "..");
-		if (dotdot) {
-			unsigned char from_sha1[20];
-			const char *next = dotdot + 2;
-			const char *this = arg;
-			*dotdot = 0;
-			if (!*next)
-				next = "HEAD";
-			if (dotdot == arg)
-				this = "HEAD";
-			if (!get_sha1(this, from_sha1) &&
-			    !get_sha1(next, sha1)) {
-				struct object *exclude;
-				struct object *include;
-
-				exclude = get_reference(revs, this, from_sha1, flags ^ UNINTERESTING);
-				include = get_reference(revs, next, sha1, flags);
-				if (!exclude || !include)
-					die("Invalid revision range %s..%s", arg, next);
-
-				if (!seen_dashdash) {
-					*dotdot = '.';
-					verify_non_filename(revs->prefix, arg);
-				}
-				add_pending_object(revs, exclude, this);
-				add_pending_object(revs, include, next);
-				continue;
-			}
-			*dotdot = '.';
-		}
-		dotdot = strstr(arg, "^@");
-		if (dotdot && !dotdot[2]) {
-			*dotdot = 0;
-			if (add_parents_only(revs, arg, flags))
-				continue;
-			*dotdot = '^';
-		}
-		local_flags = 0;
-		if (*arg == '^') {
-			local_flags = UNINTERESTING;
-			arg++;
-		}
-		if (get_sha1(arg, sha1)) {
-			int j;
-
-			if (seen_dashdash || local_flags)
-				die("bad revision '%s'", arg);
-
-			/* If we didn't have a "--":
-			 * (1) all filenames must exist;
-			 * (2) all rev-args must not be interpretable
-			 *     as a valid filename.
-			 * but the latter we have checked in the main loop.
-			 */
-			for (j = i; j < argc; j++)
-				verify_filename(revs->prefix, argv[j]);
-
-			revs->prune_data = get_pathspec(revs->prefix, argv + i);
 			break;
+		case GITOPT_NON_OPTION:
+			/* fprintf(stderr,"no: %s\n",gi.argv[gi.pos]); */
+			i = setup_revision(&gi, revs, flags, seen_dashdash);
+			gi.pos += i - 1;
+			break;
+		default:
+			/* fprintf(stderr,"do: %s\n",gi.argv[gi.pos]); */
+			i = diff_opt_handler(&gi, id, &revs->diffopt);
+			if (i > 0)
+				revs->diff = 1;
+			else if (i < 0) /* error */
+				b->argv[b->argc++] = gi.argv[gi.pos++];
+			else if (ge) {
+				/* i = 0: not a diff_opt, try other handler */
+				i = ge->opt_handler(&gi, id, ge->args);
+				if (i <= 0) /* error */
+					b->argv[b->argc++] = gi.argv[gi.pos++];
+			}
+			gi.pos += i - 1;
 		}
-		if (!seen_dashdash)
-			verify_non_filename(revs->prefix, arg);
-		object = get_reference(revs, arg, sha1, flags ^ local_flags);
-		add_pending_object(revs, object, arg);
 	}
 	if (def && !revs->pending_objects) {
 		unsigned char sha1[20];
@@ -844,7 +875,8 @@ int setup_revisions(int argc, const char
 	revs->diffopt.abbrev = revs->abbrev;
 	diff_setup_done(&revs->diffopt);
 
-	return left;
+	free(ost);
+	return b;
 }
 
 void prepare_revision_walk(struct rev_info *revs)
diff --git a/revision.h b/revision.h
index 48d7b4c..28211b6 100644
--- a/revision.h
+++ b/revision.h
@@ -81,7 +81,9 @@ extern int rev_same_tree_as_empty(struct
 extern int rev_compare_tree(struct rev_info *, struct tree *t1, struct tree *t2);
 
 extern void init_revisions(struct rev_info *revs);
-extern int setup_revisions(int argc, const char **argv, struct rev_info *revs, const char *def);
+extern struct exec_args *setup_revisions(int, const char **argv,
+					struct rev_info *revs, const char *def,
+					const struct gitopt_extra *);
 extern void prepare_revision_walk(struct rev_info *revs);
 extern struct commit *get_revision(struct rev_info *revs);
 
-- 
1.3.2.g102e322
Eric Wong· May 14, 2006, 15:19 UTC · re: Eric Wong · lore

[PATCH 4/5] commit: allow --pretty= args to be abbreviated

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 commit.c |   42 +++++++++++++++++++++++++++++-------------
 1 files changed, 29 insertions(+), 13 deletions(-)
b6190a2b46acc250277fd55e33e0a7216b7fad75
Show changes to commit.c +29 −13
diff --git a/commit.c b/commit.c
index 2343729..a786371 100644
--- a/commit.c
+++ b/commit.c
@@ -22,23 +22,39 @@ struct sort_node
 
 const char *commit_type = "commit";
 
+struct cmt_fmt_map {
+	const char *n;
+	enum cmit_fmt v;
+} cmt_fmts[] = {
+	{ "raw",	CMIT_FMT_RAW },
+	{ "medium",	CMIT_FMT_MEDIUM },
+	{ "short",	CMIT_FMT_SHORT },
+	{ "full",	CMIT_FMT_FULL },
+	{ "fuller",	CMIT_FMT_FULLER },
+	{ "oneline",	CMIT_FMT_ONELINE },
+};
+
 enum cmit_fmt get_commit_format(const char *arg)
 {
+	int i, found = -1;
 	if (!arg)
 		return CMIT_FMT_DEFAULT;
-	if (!strcmp(arg, "raw"))
-		return CMIT_FMT_RAW;
-	if (!strcmp(arg, "medium"))
-		return CMIT_FMT_MEDIUM;
-	if (!strcmp(arg, "short"))
-		return CMIT_FMT_SHORT;
-	if (!strcmp(arg, "full"))
-		return CMIT_FMT_FULL;
-	if (!strcmp(arg, "fuller"))
-		return CMIT_FMT_FULLER;
-	if (!strcmp(arg, "oneline"))
-		return CMIT_FMT_ONELINE;
-	die("invalid --pretty format");
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (!strcmp(arg, cmt_fmts[i].n))
+			return cmt_fmts[i].v;
+	}
+
+	/* look for abbreviations */
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (strstr(cmt_fmts[i].n, arg)) {
+			if (found >= 0)
+				die("invalid --pretty format: %s", arg);
+			found = i;
+		}
+	}
+	if (found >= 0)
+		return cmt_fmts[found].v;
+	die("invalid --pretty format: %s", arg);
 }
 
 static struct commit *check_commit(struct object *obj,
-- 
1.3.2.g102e322
Junio C Hamano· May 14, 2006, 23:47 UTC · re: Eric Wong · lore

Re: [PATCH 4/5] commit: allow --pretty= args to be abbreviated

Eric Wong <normalperson@yhbt.net> writes:
>  commit.c |   42 +++++++++++++++++++++++++++++-------------
>  1 files changed, 29 insertions(+), 13 deletions(-)

This is applicable without the gitopt changes, but I have a feeling that when we think about abbreviations the users would expect the leading substring abbreviation, not strstr().

While "git log --pretty=lle" or "git log --pretty=or" might be unambiguous, I think that is trying to be too cute and confusing, especially if somebody picks up that habit by watching others type such a cute abbreviations.

That comment probably incidentally applies to your bigger patches.

Eric Wong· May 15, 2006, 00:34 UTC · re: Junio C Hamano · lore

[PATCH] commit: allow --pretty= args to be abbreviated

Unlike the original one, this one only does prefix matches, so you can't do --pretty=er anymore :)

This one really works with and without the gitopt changes.
Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
Junio C Hamano <junkio@cox.net> wrote:
Show 16 quoted lines
> Eric Wong <normalperson@yhbt.net> writes:
> 
> >  commit.c |   42 +++++++++++++++++++++++++++++-------------
> >  1 files changed, 29 insertions(+), 13 deletions(-)
> 
> This is applicable without the gitopt changes, but I have a
> feeling that when we think about abbreviations the users would
> expect the leading substring abbreviation, not strstr().
> 
> While "git log --pretty=lle" or "git log --pretty=or" might be
> unambiguous, I think that is trying to be too cute and
> confusing, especially if somebody picks up that habit by
> watching others type such a cute abbreviations.
> 
> That comment probably incidentally applies to your bigger
> patches.

Ok. The current gitopt patch only uses leading substring abbreviations by default. GIT_ABBREV_HARDER needs to be set if you want all the crazyness :)

 commit.c |   50 ++++++++++++++++++++++++++++++++++++--------------
 1 files changed, 36 insertions(+), 14 deletions(-)
d4473559924b1a5ba655cd5d5b18d39f53e25184
Show changes to commit.c +36 −14
diff --git a/commit.c b/commit.c
index 2717dd8..2753677 100644
--- a/commit.c
+++ b/commit.c
@@ -22,23 +22,45 @@ struct sort_node
 
 const char *commit_type = "commit";
 
+struct cmt_fmt_map {
+	const char *n;
+	enum cmit_fmt v;
+} cmt_fmts[] = {
+	{ "raw",	CMIT_FMT_RAW },
+	{ "medium",	CMIT_FMT_MEDIUM },
+	{ "short",	CMIT_FMT_SHORT },
+	{ "full",	CMIT_FMT_FULL },
+	{ "fuller",	CMIT_FMT_FULLER },
+	{ "oneline",	CMIT_FMT_ONELINE },
+};
+
 enum cmit_fmt get_commit_format(const char *arg)
 {
-	if (!*arg)
+	int i, found;
+	size_t len;
+
+	if (!arg || !*arg)
 		return CMIT_FMT_DEFAULT;
-	if (!strcmp(arg, "=raw"))
-		return CMIT_FMT_RAW;
-	if (!strcmp(arg, "=medium"))
-		return CMIT_FMT_MEDIUM;
-	if (!strcmp(arg, "=short"))
-		return CMIT_FMT_SHORT;
-	if (!strcmp(arg, "=full"))
-		return CMIT_FMT_FULL;
-	if (!strcmp(arg, "=fuller"))
-		return CMIT_FMT_FULLER;
-	if (!strcmp(arg, "=oneline"))
-		return CMIT_FMT_ONELINE;
-	die("invalid --pretty format");
+	if (*arg == '=')
+		arg++;
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (!strcmp(arg, cmt_fmts[i].n))
+			return cmt_fmts[i].v;
+	}
+
+	/* look for abbreviations */
+	len = strlen(arg);
+	found = -1;
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (!strncmp(cmt_fmts[i].n, arg, len)) {
+			if (found >= 0)
+				die("invalid --pretty format: %s", arg);
+			found = i;
+		}
+	}
+	if (found >= 0)
+		return cmt_fmts[found].v;
+	die("invalid --pretty format: %s", arg);
 }
 
 static struct commit *check_commit(struct object *obj,
-- 
1.3.2.g58c0
Junio C Hamano· May 15, 2006, 20:47 UTC · re: Eric Wong · lore

Re: [PATCH] commit: allow --pretty= args to be abbreviated

Eric Wong <normalperson@yhbt.net> writes:
> Unlike the original one, this one only does prefix matches, so
> you can't do --pretty=er anymore :)

Sounds good. But then you know how long the unique prefix are for each candidate, so wouldn't this rather be redundant, I wonder?

Show 15 quoted lines
> +
> +	/* look for abbreviations */
> +	len = strlen(arg);
> +	found = -1;
> +	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
> +		if (!strncmp(cmt_fmts[i].n, arg, len)) {
> +			if (found >= 0)
> +				die("invalid --pretty format: %s", arg);
> +			found = i;
> +		}
> +	}
> +	if (found >= 0)
> +		return cmt_fmts[found].v;
> +	die("invalid --pretty format: %s", arg);
>  }

It would probably be better to say "ambiguous" not "invalid" in the die() message.

Eric Wong· May 16, 2006, 13:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] commit: allow --pretty= args to be abbreviated

Junio C Hamano <junkio@cox.net> wrote:
Show 8 quoted lines
> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Unlike the original one, this one only does prefix matches, so
> > you can't do --pretty=er anymore :)
> 
> Sounds good.  But then you know how long the unique prefix
> are for each candidate, so wouldn't this rather be redundant, I
> wonder?

I just copied the idea for the 2nd for loop from gitopt, since it makes maintenance easier when there are lots of possibilities. We only have 6 (soon 7) to worry about for --pretty= here, so hard coding lengths probably makes more sense.

Show 18 quoted lines
> > +
> > +	/* look for abbreviations */
> > +	len = strlen(arg);
> > +	found = -1;
> > +	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
> > +		if (!strncmp(cmt_fmts[i].n, arg, len)) {
> > +			if (found >= 0)
> > +				die("invalid --pretty format: %s", arg);
> > +			found = i;
> > +		}
> > +	}
> > +	if (found >= 0)
> > +		return cmt_fmts[found].v;
> > +	die("invalid --pretty format: %s", arg);
> >  }
> 
> It would probably be better to say "ambiguous" not "invalid" in
> the die() message.
Yes, but only one die() message left now :)
-- 
Eric Wong

>From nobody Mon Sep 17 00:00:00 2001
From: Eric Wong <normalperson@yhbt.net>
Date: Sun, 14 May 2006 17:20:46 -0700
Subject: [PATCH] commit: allow --pretty= args to be abbreviated

Unlike the original one, this one only does prefix matches, so
you can't do --pretty=er anymore :)

This one really works with and without the gitopt changes.

Signed-off-by: Eric Wong <normalperson@yhbt.net>

---

 commit.c |   38 ++++++++++++++++++++++++--------------
 1 files changed, 24 insertions(+), 14 deletions(-)

044dff6523c25e173eb7fb1c5d5c8a8e6ada8fdc
diff --git a/commit.c b/commit.c
index 2717dd8..4a26070 100644
--- a/commit.c
+++ b/commit.c
@@ -22,23 +22,33 @@ struct sort_node
 
 const char *commit_type = "commit";
 
+struct cmt_fmt_map {
+	const char *n;
+	size_t cmp_len;
+	enum cmit_fmt v;
+} cmt_fmts[] = {
+	{ "raw",	1,	CMIT_FMT_RAW },
+	{ "medium",	1,	CMIT_FMT_MEDIUM },
+	{ "short",	1,	CMIT_FMT_SHORT },
+	{ "full",	5,	CMIT_FMT_FULL },
+	{ "fuller",	5,	CMIT_FMT_FULLER },
+	{ "oneline",	1,	CMIT_FMT_ONELINE },
+};
+
 enum cmit_fmt get_commit_format(const char *arg)
 {
-	if (!*arg)
+	int i;
+
+	if (!arg || !*arg)
 		return CMIT_FMT_DEFAULT;
-	if (!strcmp(arg, "=raw"))
-		return CMIT_FMT_RAW;
-	if (!strcmp(arg, "=medium"))
-		return CMIT_FMT_MEDIUM;
-	if (!strcmp(arg, "=short"))
-		return CMIT_FMT_SHORT;
-	if (!strcmp(arg, "=full"))
-		return CMIT_FMT_FULL;
-	if (!strcmp(arg, "=fuller"))
-		return CMIT_FMT_FULLER;
-	if (!strcmp(arg, "=oneline"))
-		return CMIT_FMT_ONELINE;
-	die("invalid --pretty format");
+	if (*arg == '=')
+		arg++;
+	for (i = 0; i < ARRAY_SIZE(cmt_fmts); i++) {
+		if (!strncmp(arg, cmt_fmts[i].n, cmt_fmts[i].cmp_len))
+			return cmt_fmts[i].v;
+	}
+
+	die("invalid --pretty format: %s", arg);
 }
 
 static struct commit *check_commit(struct object *obj,
-- 
1.3.2.g7d11
Eric Wong· May 14, 2006, 15:19 UTC · re: Eric Wong · lore

[PATCH 5/5] diff: parse U/u/unified options with an optional integer arg

Looks like Linus and I both think GIT_DIFF_OPTS="--unified=5" is silly, but it still continues to work.

This was originally bundled into my first series gitopt patches, and now Linus has a more correct/complete one that affects combine-diff and works with uppercase -U This patch combines the Linus one with my gitopt version.

-u (lowercase) now accepts an optional arg, like -U (GNU diff -u also has this behavior).

This uses the built-in gitopt parsers to do optional integer argument handling.

Signed-off-by: Eric Wong <normalperson@yhbt.net>
---
 combine-diff.c |    1 +
 diff.c         |   11 ++++++++---
 diff.h         |    1 +
 3 files changed, 10 insertions(+), 3 deletions(-)
102e3227de7c7f08a69096d5094ee31128bf9819
Show changes to 3 files +10 −3

combine-diff.c, diff.c, diff.h

diff --git a/combine-diff.c b/combine-diff.c
index 8a8fe38..64b20cc 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -608,6 +608,7 @@ static int show_patch_diff(struct combin
 	int abbrev = opt->full_index ? 40 : DEFAULT_ABBREV;
 	mmfile_t result_file;
 
+	context = opt->context;
 	/* Read the result of merge first */
 	if (!working_tree_file)
 		result = grab_blob(elem->sha1, &result_size);
diff --git a/diff.c b/diff.c
index 7d88dc5..6ee612f 100644
--- a/diff.c
+++ b/diff.c
@@ -15,7 +15,7 @@ #include "gitopt.h"
 static int diff_scoreopt_parse(const int id, const char *opt);
 
 enum diff_ost_ids {
-	opt_p = GITOPT_DIFF_BASE, opt_raw, opt_patch_with_raw,
+	opt_u = GITOPT_DIFF_BASE, opt_p, opt_raw, opt_patch_with_raw,
 	opt_stat, opt_patch_with_stat,
 	opt_z, opt_l, opt_full_index, opt_name_only, opt_name_status,
 	opt_R, opt_S, opt_s, opt_O, opt_diff_filter,
@@ -26,7 +26,8 @@ enum diff_ost_ids {
 
 const struct opt_spec diff_ost[] = {
 	{ 0,			'p',	0,	0,	opt_p },
-	{ "unified",		'u',	0,	0,	opt_p },
+	{ "unified",		'u',	0,	ARG_OPTINT,	opt_u },
+	{ 0,			'U',	0,	ARG_OPTINT,	opt_u },
 	{ "raw",		0,	0,	0,	opt_raw },
 	{ "patch-with-raw",	0,	0,	0,	opt_patch_with_raw },
 	{ "stat",		0,	0,	0,	opt_stat },
@@ -600,7 +601,7 @@ static void builtin_diff(const char *nam
 
 		ecbdata.label_path = lbl;
 		xpp.flags = XDF_NEED_MINIMAL;
-		xecfg.ctxlen = 3;
+		xecfg.ctxlen = o->context;
 		xecfg.flags = XDL_EMIT_FUNCNAMES;
 		if (!diffopts)
 			;
@@ -1224,6 +1225,7 @@ void diff_setup(struct diff_options *opt
 	options->line_termination = '\n';
 	options->break_opt = -1;
 	options->rename_limit = -1;
+	options->context = 3;
 
 	options->change = diff_change;
 	options->add_remove = diff_addremove;
@@ -1269,6 +1271,9 @@ int diff_opt_handler(struct gitopt_itera
 	struct diff_options *options = (struct diff_options *)args;
 
 	switch (id) {
+	case opt_u:
+		if (gi->ea->argc > 1)
+			options->context = strtol(gi->ea->argv[1], NULL, 10);
 	case opt_p:
 		options->output_format = DIFF_FORMAT_PATCH;
 		break;
diff --git a/diff.h b/diff.h
index fa44d1b..b9d7573 100644
--- a/diff.h
+++ b/diff.h
@@ -33,6 +33,7 @@ struct diff_options {
 		 full_index:1,
 		 silent_on_remove:1,
 		 find_copies_harder:1;
+	int context;
 	int break_opt;
 	int detect_rename;
 	int line_termination;
-- 
1.3.2.g102e322
Linus Torvalds· May 14, 2006, 16:33 UTC · re: Eric Wong · lore

Re: [PATCH 5/5] diff: parse U/u/unified options with an optional integer arg

On Sun, 14 May 2006, Eric Wong wrote:
> 
> -u (lowercase) now accepts an optional arg, like -U (GNU diff
> -u also has this behavior).
Actually, modern GNU "diff -u5" will say
	diff: `-5' option is obsolete; use `-U 5'
	diff: Try `diff --help' for more information.
and exit.

I'm not entirely sure why, but I think it's because "u" can be mixed (ie with something like "-urN"), while "U" cannot. The GNU diff rule seems to be that simple arguments can be mixed together, but arguments with parameters cannot. Which sounds sane.

		Linus
Eric Wong· May 14, 2006, 23:36 UTC · re: Linus Torvalds · lore

Re: [PATCH 5/5] diff: parse U/u/unified options with an optional integer arg

Linus Torvalds <torvalds@osdl.org> wrote:
Show 18 quoted lines
> 
> 
> On Sun, 14 May 2006, Eric Wong wrote:
> > 
> > -u (lowercase) now accepts an optional arg, like -U (GNU diff
> > -u also has this behavior).
> 
> Actually, modern GNU "diff -u5" will say
> 
> 	diff: `-5' option is obsolete; use `-U 5'
> 	diff: Try `diff --help' for more information.
> 
> and exit.
> 
> I'm not entirely sure why, but I think it's because "u" can be mixed (ie 
> with something like "-urN"), while "U" cannot. The GNU diff rule seems to 
> be that simple arguments can be mixed together, but arguments with 
> parameters cannot. Which sounds sane.

Interesting. With _POSIX2_VERSION < 200112, diff is happy to accept -u5. Setting _POSIX2_VERSION >= 200112 causes it to burp, however.

It turns out -u5 is actually short for -u -5, but I (and apparently some other people) have always assumed -u5 worked like -U5. POSIX doesn't like -<num> switches, however, but we support it regardless in rev-list.

I prefer that it works the -u5 way, of course :)

Also, I find integer arguments being bundled with other parameters to be pretty nice feature, but it's easy to disable if users don't like it:

Show changes to gitopt.c +3 −11
diff --git a/gitopt.c b/gitopt.c
index 9e85247..139cd2d 100644
--- a/gitopt.c
+++ b/gitopt.c
@@ -270,17 +270,9 @@ static const char * parse_bundled(struct
        } else if (s->arg_fl) {
                if (*cur) {
                        /* no space between the arg and opt switch: */
-                       if (s->arg_fl & ARG_IS_INT) {
-                               /* we know to handle stuff like:
-                                * -h24w80 => -h=24 -w=80 */
-                               char *endptr = (char *)cur;
-                               strtol(cur, &endptr, 10);
-
-                               while (cur < endptr)
-                                       *c++ = *cur++;
-                       } else if (s->arg_fl & ARG_ONE) {
-                               /* unfortunately, other args are less
-                                * clear-cut */
+                       if (s->arg_fl & ARG_ONE) {
+                               /* don't attempt further unbundling, extra
+                                * chars are arguments */
                                while (*cur)
                                        *c++ = *cur++;
                        }
-- 
Eric Wong

← back to recent threads