[PATCH v7 3/3] rev-parse: stop interpreting flags as options to rev-parse once --flags is specified

[Jon Seymour <jon.seymour@gmail.com>]

Current git rev-parse behaviour makes --flags hard to use if the remaining
arguments to git rev-parse contain an option that would otherwise be interpreted
as an option by git rev-parse itself.

So, for example:

  $> git rev-parse --flags -q -X
  -X

Normally one might expect to use -- to prevent -q being interpreted:

  $> git rev-parse --flags -- -q -X
  -q -X

But we can't really use -- in this way, because commands that use
git rev-parse might reasonably expect:

  $> git rev-parse --flags -Y -- -q -X
  -Y

That is, -Y to be regarded as a flag but everything after -- to be uninterpreted.

This proposed change modifies git rev-parse so that git rev-parse stops
interpreting flag arguments as options to git rev-parse once --flags is
interpreted. We also exit early once -- is found.

Previously:
 $> git rev-parse --flags --all
 {list of sha1 hashes}
 $>

Now:
 $> git rev-parse --flags --all
 --all
 $>

Signed-off-by: Jon Seymour <jon.seymour@gmail.com>
---
 Documentation/git-rev-parse.txt |   11 +++++++++--
 builtin/rev-parse.c             |    8 ++++++++
 t/t1510-rev-parse-flags.sh      |   14 ++++++--------
 3 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 27d15b0..3eac735 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -51,11 +51,18 @@ OPTIONS
 --flags::
 	Output any flag and revision-like values in the remaining parameters.
 +
-Note that any parameter which is also a valid 'git rev-parse' option
-will be interpreted as an option to 'git rev-parse' and thus will not be output.
+If specified, this option causes 'git rev-parse' to stop
+interpreting remaining arguments as options for its own
+consumption. As such, this option should be specified
+after all other options that 'git rev-parse' is expected
+to interpret.
 
 --no-flags::
 	Do not output flag parameters.
++
+If both `--flags` and `--no-flags` are specified, the first
+option specified wins and the other option is treated like
+a non-option argument.
 
 --default <arg>::
 	If there is no parameter given by the user, use `<arg>`
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index a5a1c86..2ad269a 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -497,8 +497,16 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				/* Pass on the "--" if we show anything but files.. */
 				if (filter & (DO_FLAGS | DO_REVS))
 					show_file(arg);
+				if (!(filter & DO_NONFLAGS)) {
+					return 0;
+				}
 				continue;
 			}
+			if (!(filter & DO_NONFLAGS)) {
+				/* once we see --flags, we stop interpreting other flags */
+				 show_flag(arg);
+				 continue;
+			}
 			if (!strcmp(arg, "--default")) {
 				def = argv[i+1];
 				i++;
diff --git a/t/t1510-rev-parse-flags.sh b/t/t1510-rev-parse-flags.sh
index e327b96..1e8311e 100755
--- a/t/t1510-rev-parse-flags.sh
+++ b/t/t1510-rev-parse-flags.sh
@@ -92,16 +92,16 @@ test_expect_success 'git rev-parse --symbolic --no-flags --flags HEAD -> "HEAD"'
 	test_cmp expected actual
 '
 
-test_expect_success 'git rev-parse --flags -q -> ""' \
+test_expect_success 'git rev-parse --flags -q -> "-q"' \
 '
-	>expected &&
+	printf "%s\n" -q > expected &&
 	git rev-parse --flags -q >actual &&
 	test_cmp expected actual
 '
 
-test_expect_success 'git rev-parse --flags --no-flags -> ""' \
+test_expect_success 'git rev-parse --flags --no-flags -> "--no-flags"' \
 '
-	>expected &&
+	printf "%s\n" --no-flags > expected &&
 	git rev-parse --flags --no-flags >actual &&
 	test_cmp expected actual
 '
@@ -134,10 +134,8 @@ not-a-file
 
 test_expect_success 'git rev-parse --flags --all -> list of revs' \
 '
-	cat >expected <<-EOF &&
-commit
-	EOF
-	git cat-file -t $(git rev-parse --flags --all | head -1) >actual &&
+	printf "%s\n" "--all" >expected &&
+	git rev-parse --flags --all >actual &&
 	test_cmp expected actual
 '
 
-- 
1.7.3.3.g9129b6