[WRONG/PATCH 1/3] revisions: clarify handling of --no-walk and --do-walk

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 21, 2011, 10:39 UTC
Message-ID: <20110421103926.GA16260@elie>
In-Reply-To: <20110421102241.GA16185@elie>

As v1.6.0-rc2~42 (Allow "non-option" revision options in parse_option-enabled commands, 2008-07-31) explains, commands which use parse_options() but also call setup_revisions() do their parsing in two stages:

 1. first, they parse all options. Anything unknown goes to
    parse_revision_opt() (which calls handle_revision_opt), which
    may claim the option or say "I don't recognize this"

 2. the non-option remainder goes to setup_revisions() to
    actually get turned into revisions

Some revision options, like --all and --not, are "non-options" in that they must be parsed in order with their revision counterparts in setup_revisions(). It would be nice if --no-walk and --do-walk fell in this category and set a flag only for revs coming after them on the command line, but they do not, so move parsing of --no-walk and --do-walk to the first "global options" stage for clarity.

--- Wait, the above is not actually the full story. If I do

	git show maint..master

then this turns on walking automatically, to give the commit range meaning. Likewise

	git log --no-walk maint..master

will, in fact walk, but

	git log maint..master --no-walk

will not. Which I should have understood from v1.6.0-rc2~42 (2008-07-31) already. Will think more; sorry for the nonsense.

 revision.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/revision.c b/revision.c
index 0f38364..7b87bd0 100644
--- a/revision.c
+++ b/revision.c
@@ -1177,7 +1177,6 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 	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") ||
 	    !strcmp(arg, "--bisect"))
 	{
 		unkv[(*unkc)++] = arg;
@@ -1334,6 +1333,10 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 		revs->unpacked = 1;
 	} else if (!prefixcmp(arg, "--unpacked=")) {
 		die("--unpacked=<packfile> no longer supported.");
+	} else if (!strcmp(arg, "--no-walk")) {
+		revs->no_walk = 1;
+	} else if (!strcmp(arg, "--do-walk")) {
+		revs->no_walk = 0;
 	} else if (!strcmp(arg, "-r")) {
 		revs->diff = 1;
 		DIFF_OPT_SET(&revs->diffopt, RECURSIVE);
@@ -1622,14 +1625,6 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s
 				flags ^= UNINTERESTING;
 				continue;
 			}
-			if (!strcmp(arg, "--no-walk")) {
-				revs->no_walk = 1;
-				continue;
-			}
-			if (!strcmp(arg, "--do-walk")) {
-				revs->no_walk = 0;
-				continue;
-			}
 			if (!strcmp(arg, "--stdin")) {
 				if (revs->disable_stdin) {
 					argv[left++] = arg;

-- 
1.7.5.rc3