Re: [PATCH v3 1/2] rev-parse: add --prefix option

[Ramkumar Ramachandra <artagnon@gmail.com>]

John Keeping wrote:
> Changes since v2:
>     - Rewrite commit message
>     - Add a new test for 'prefix ignored with HEAD:top'
>     - Use '<<-\EOF' where appropriate in t1513

Thanks for the re-roll.

In the previous iteration, I wasn't sure this was the right approach
because I thought it would be better to pass RUN_SETUP and let
run_builtin_command() take care of the prefix-setting.  Unfortunately,
as 5410a02 (git-rev-parse --parseopt, 2007-11-06) indicates, we have
to run setup_git_directory() in cmd_rev_parse() after parsing
--parseopt, as 'git rev-parse --parseopt' can be run outside a git
repository.  You might want to include this note in your commit
message for the benefit of other readers.

Other than that, I just have one small suggestion: it's possible to
avoid passing output_prefix around, and simplify show_file() a bit if
we acknowledge that printing "--" is not the same as printing a file
(although the condition is the same).  Would you like to squash this
in?

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
-- 8< --
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index de894c7..7e69b3f 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -29,6 +29,7 @@ static int abbrev;
 static int abbrev_ref;
 static int abbrev_ref_strict;
 static int output_sq;
+static int output_prefix;
 
 /*
  * Some arguments are relevant "revision" arguments,
@@ -212,15 +213,13 @@ static void show_datestring(const char *flag, const char *datestr)
 	show(buffer);
 }
 
-static int show_file(const char *arg, int output_prefix)
+static int show_file(const char *arg)
 {
 	show_default();
 	if ((filter & (DO_NONFLAGS|DO_NOREV)) == (DO_NONFLAGS|DO_NOREV)) {
 		if (output_prefix) {
 			const char *prefix = startup_info->prefix;
-			show(prefix_filename(prefix,
-					     prefix ? strlen(prefix) : 0,
-					     arg));
+			show(prefix_filename(prefix, strlen(prefix), arg));
 		} else
 			show(arg);
 		return 1;
@@ -228,6 +227,16 @@ static int show_file(const char *arg, int output_prefix)
 	return 0;
 }
 
+static int show_dashdash()
+{
+	show_default();
+	if ((filter & (DO_NONFLAGS | DO_NOREV)) == (DO_NONFLAGS | DO_NOREV)) {
+		show("--");
+		return 1;
+	}
+	return 0;
+}
+
 static int try_difference(const char *arg)
 {
 	char *dotdot;
@@ -476,7 +485,6 @@ N_("git rev-parse --parseopt [options] -- [<args>...]\n"
 int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 {
 	int i, as_is = 0, verify = 0, quiet = 0, revs_count = 0, type = 0;
-	int output_prefix = 0;
 	unsigned char sha1[20];
 	const char *name = NULL;
 
@@ -510,7 +518,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 		const char *arg = argv[i];
 
 		if (as_is) {
-			if (show_file(arg, output_prefix) && as_is < 2)
+			if (show_file(arg) && as_is < 2)
 				verify_filename(prefix, arg, 0);
 			continue;
 		}
@@ -534,7 +542,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				as_is = 2;
 				/* Pass on the "--" if we show anything but files.. */
 				if (filter & (DO_FLAGS | DO_REVS))
-					show_file(arg, 0);
+					show_dashdash();
 				continue;
 			}
 			if (!strcmp(arg, "--default")) {
@@ -543,7 +551,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!strcmp(arg, "--prefix")) {
-				prefix = argv[i+1];
+				prefix = argv[i + 1];
 				startup_info->prefix = prefix;
 				output_prefix = 1;
 				i++;
@@ -768,7 +776,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 		if (verify)
 			die_no_single_rev(quiet);
 		as_is = 1;
-		if (!show_file(arg, output_prefix))
+		if (!show_file(arg))
 			continue;
 		verify_filename(prefix, arg, 1);
 	}