[PATCH 3/6] rev-parse: fix "--" detection when it is an option value

[Christian Couder <christian.couder@gmail.com>]

`cmd_rev_parse()` walks its arguments twice. The second loop actually
parses the options, and it knows that `--default`, `--prefix` and
`--resolve-git-dir` take their value as a separate argument, so it skips
that value.

The first loop, which only looks for the "--" separating revisions from
paths, doesn't know about these options. So when such an option is given
"--" as its value, that "--" is mistaken for the separator and
`has_dashdash` is wrongly set.

This matters because `has_dashdash` makes the second loop die with a
"bad revision" error on an argument that is neither a revision nor an
existing file, instead of reporting that the argument is ambiguous and
telling how to disambiguate it. So:

  $ git rev-parse --default -- notarev
  fatal: bad revision 'notarev'

while the very same command line with any other default value gives the
usual, much more helpful, "ambiguous argument" error.

Let's fix this the same way as in a previous commit, by using
early_scan_options() and telling it about the options taking their value
as a separate argument.

Signed-off-by: Christian Couder <christian.couder@gmail.com>
---
 builtin/rev-parse.c  | 26 ++++++++++++++++++++------
 t/t1500-rev-parse.sh |  5 +++++
 2 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 43693454d5..7ced82e25d 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -695,6 +695,17 @@ static void print_path(const char *path, const char *prefix,
 	strbuf_release(&sb);
 }
 
+/*
+ * The options taking their value as a separate argument, which the scan
+ * looking for "--" below has to skip along with their value.
+ */
+static const struct early_scan_option rev_parse_early_options[] = {
+	EARLY_SCAN_SKIP_VALUE("default"),
+	EARLY_SCAN_SKIP_VALUE("prefix"),
+	EARLY_SCAN_SKIP_VALUE("resolve-git-dir"),
+	EARLY_SCAN_END()
+};
+
 int cmd_rev_parse(int argc,
 		  const char **argv,
 		  const char *prefix,
@@ -724,12 +735,15 @@ int cmd_rev_parse(int argc,
 	if (argc > 1 && !strcmp("-h", argv[1]))
 		usage(builtin_rev_parse_usage);
 
-	for (i = 1; i < argc; i++) {
-		if (!strcmp(argv[i], "--")) {
-			has_dashdash = 1;
-			break;
-		}
-	}
+	/*
+	 * The scan below has to know about the options taking their value
+	 * as a separate argument, or such a value that happens to be "--"
+	 * would be mistaken for the "--" separating revisions from paths.
+	 */
+	i = early_scan_options(argc - 1, argv + 1, rev_parse_early_options,
+			       EARLY_SCAN_STOP_AT_DASHDASH, NULL, NULL);
+	if (i < argc - 1)
+		has_dashdash = 1;
 
 	/* No options; just report on whether we're in a git repo or not. */
 	if (argc == 1) {
diff --git a/t/t1500-rev-parse.sh b/t/t1500-rev-parse.sh
index 4174ca40c3..897e9a7735 100755
--- a/t/t1500-rev-parse.sh
+++ b/t/t1500-rev-parse.sh
@@ -383,4 +383,9 @@ test_expect_success ':/ and HEAD^{/} favor more recent matching commits' '
 	)
 '
 
+test_expect_success 'rev-parse with "--" as an option value' '
+	test_must_fail git rev-parse --default -- notarev 2>err &&
+	test_grep "ambiguous argument .notarev." err
+'
+
 test_done
-- 
2.55.0.787.g3f9e2241eb.dirty