Re: [PATCH 2/6] revisions: split the pure option parsing out from parse_revisions.

From

Pierre Habouzit <madcoder@debian.org>

Date

Jul 8, 2008, 11:25 UTC

Message-ID

<20080708112532.GG19202@artemis.madism.org>

In-Reply-To

<20080708110022.GE19202@artemis.madism.org>

On Tue, Jul 08, 2008 at 11:00:22AM +0000, Pierre Habouzit wrote:

> On Tue, Jul 08, 2008 at 10:51:13AM +0000, Johannes Sixt wrote:
> > Pierre Habouzit schrieb:
> > > +int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
> > > +			int *unkc, const char **unkv)
> > > +{
> > > +	const char *arg = argv[0];
> > > +
> > > +	/* pseudo revision arguments */
> > > +	if (!strcmp(arg, "--all") || !strcmp(arg, "--branches") ||
> > > +	    !strcmp(arg, "--tags") || !strcmp(arg, "--remotes") ||
> > > +	    !strcmp(arg, "--reflog") || !strcmp(arg, "--not") ||
> > > +	    !strcmp(arg, "--no-walk") || !strcmp(arg, "--do-walk"))
> > > +	{
> > > +		unkv[*unkc++] = arg;
> > 
> > +		unkv[(*unkc)++] = arg;
> 
>   Huh right. Good catch, I wonder why the testsuite failed to see that.

  Okay, I know why, there is another bug that was hiding this one,
parse_revisions whas doing argv[left++] = arg itself too, which was
fixing this mistake. I've pushed it again, and here is the fixed patch
3/6 for reviewing.

-----------8<-----------
From d0a062617526128c4ec0245614eb9916b7996c38 Mon Sep 17 00:00:00 2001
From: Pierre Habouzit <madcoder@debian.org>
Date: Tue, 8 Jul 2008 11:00:02 +0200
Subject: [PATCH] revisions: split handle_revision_args from parse_revisions.

Signed-off-by: Pierre Habouzit <madcoder@debian.org>
---
 revision.c |   97 +++++++++++++++++++++++++++++++++++------------------------
 revision.h |    1 +
 2 files changed, 58 insertions(+), 40 deletions(-)

diff --git a/revision.c b/revision.c
index 99b3cc9..6958725 100644
--- a/revision.c
+++ b/revision.c
@@ -1241,39 +1241,15 @@ int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
 	return 1;
 }
 
-/*
- * Parse revision information, filling in the "rev_info" structure,
- * and removing the used arguments from the argument list.
- *
- * Returns the number of arguments left that weren't recognized
- * (which are also moved to the head of the argument list)
- */
-int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+int handle_revision_args(struct rev_info *revs,
+			 int argc, const char **argv, int dashdash_pos)
 {
-	int i, flags, seen_dashdash;
-	const char **unrecognized = argv + 1;
-	int left = 1;
+	int i, left, flags = 0;
 
-	/* First, search for "--" */
-	seen_dashdash = 0;
-	for (i = 1; i < argc; i++) {
+	for (left = i = 1; i < argc; i++) {
 		const char *arg = argv[i];
-		if (strcmp(arg, "--"))
-			continue;
-		argv[i] = NULL;
-		argc = i;
-		if (argv[i + 1])
-			revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
-		seen_dashdash = 1;
-		break;
-	}
 
-	flags = 0;
-	for (i = 1; i < argc; i++) {
-		const char *arg = argv[i];
 		if (*arg == '-') {
-			int opts;
-
 			if (!strcmp(arg, "--all")) {
 				handle_refs(revs, flags, for_each_ref);
 				continue;
@@ -1306,23 +1282,22 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
 				revs->no_walk = 0;
 				continue;
 			}
-
-			opts = handle_revision_opt(revs, argc - i, argv + i, NULL, NULL);
-			if (opts > 0) {
-				i += opts - 1;
-				continue;
+			if (i == dashdash_pos) {
+				argv[i] = NULL;
+				argc = i;
+				if (argv[i + 1])
+					revs->prune_data = get_pathspec(revs->prefix, argv + i + 1);
+				break;
 			}
-			if (opts < 0)
-				exit(128);
-			*unrecognized++ = arg;
-			left++;
+
+			argv[left++] = arg;
 			continue;
 		}
 
-		if (handle_revision_arg(arg, revs, flags, seen_dashdash)) {
+		if (handle_revision_arg(arg, revs, flags, dashdash_pos)) {
 			int j;
-			if (seen_dashdash || *arg == '^')
-				die("bad revision '%s'", arg);
+			if (dashdash_pos || *arg == '^')
+				return error("bad revision '%s'", arg);
 
 			/* If we didn't have a "--":
 			 * (1) all filenames must exist;
@@ -1338,9 +1313,51 @@ int parse_revisions(int argc, const char **argv, struct rev_info *revs)
 			break;
 		}
 	}
+
 	return left;
 }
 
+/*
+ * Parse revision information, filling in the "rev_info" structure,
+ * and removing the used arguments from the argument list.
+ *
+ * Returns the number of arguments left that weren't recognized
+ * (which are also moved to the head of the argument list)
+ */
+int parse_revisions(int argc, const char **argv, struct rev_info *revs)
+{
+	int i, left, dashdash_pos = 0;
+
+	/* First, filter out revision options and look for "--" */
+	for (left = i = 1; i < argc; i++) {
+		const char *arg = argv[i];
+		int opts;
+
+		if (arg[0] != '-') {
+			argv[left++] = arg;
+			continue;
+		}
+
+		if (!strcmp(arg, "--")) {
+			dashdash_pos = left;
+			memcpy(argv + left, argv + i, sizeof(*argv) * (argc - i));
+			left += argc - i;
+			break;
+		}
+
+		opts = handle_revision_opt(revs, argc - i, argv + i, &left, argv);
+		if (opts > 0) {
+			i += opts - 1;
+			continue;
+		}
+		if (opts < 0)
+			exit(128);
+	}
+	argv[left] = NULL;
+
+	return handle_revision_args(revs, left, argv, dashdash_pos);
+}
+
 void setup_revisions(struct rev_info *revs, const char *def)
 {
 	if (revs->def == NULL)
diff --git a/revision.h b/revision.h
index 46ab713..c0f5df0 100644
--- a/revision.h
+++ b/revision.h
@@ -125,6 +125,7 @@ extern int parse_revisions(int argc, const char **argv, struct rev_info *revs);
 extern void setup_revisions(struct rev_info *revs, const char *def);
 extern int handle_revision_opt(struct rev_info *revs, int argc, const char **argv,
 			       int *unkc, const char **unkv);
+extern int handle_revision_args(struct rev_info *revs, int argc, const char **argv, int dashdash_pos);
 extern int handle_revision_arg(const char *arg, struct rev_info *revs,int flags,int cant_be_filename);
 
 extern int prepare_revision_walk(struct rev_info *revs);

-- 
1.5.6.2.399.g15e15