[RFC PATCH 1/2] rev-list: handle arguments in early option scan

[Azuama Odinaka Maximus <azuama69@gmail.com>]

The early option scan in cmd_rev_list() looks for options that
affect missing-object handling before setup_revisions() parses the
command line. It must not interpret an option's separate argument
as another option.

Use parse_long_opt() to recognize revision options that take
separate arguments, and skip those arguments. Handle -n and
--default as well. Stop scanning at --, where subsequent arguments
are pathspecs.

Add tests to ensure that --grep and --author arguments beginning
with -- are not mistaken for options during the early scan.

Signed-off-by: Azuama Odinaka Maximus <azuama69@gmail.com>
---
 builtin/rev-list.c          | 55 +++++++++++++++++++++++++++++++++++--
 t/t6022-rev-list-missing.sh | 26 ++++++++++++++++++
 2 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/builtin/rev-list.c b/builtin/rev-list.c
index 071a0e3cb5..4dd646050a 100644
--- a/builtin/rev-list.c
+++ b/builtin/rev-list.c
@@ -744,6 +744,47 @@ static void prepare_maximal_independent(struct rev_info *revs)
 	revs->no_walk = 1;
 }
 
+static int rev_list_option_takes_argument(const char **argv)
+{
+	static const char * const options[] = {
+		"max-count",
+		"max-count-oldest",
+		"skip",
+		"max-age",
+		"since",
+		"since-as-filter",
+		"after",
+		"min-age",
+		"before",
+		"until",
+		"date",
+		"author",
+		"committer",
+		"grep-reflog",
+		"grep",
+		"encoding",
+		"glob",
+		"exclude",
+		"exclude-hidden",
+	};
+	const char *optarg;
+	size_t i;
+
+	if (!argv[1])
+		return 0;
+
+	if (!strcmp(argv[0], "-n") ||
+	    !strcmp(argv[0], "--default"))
+		return 1;
+
+	for (i = 0; i < ARRAY_SIZE(options); i++) {
+		if (parse_long_opt(options[i], argv, &optarg) == 2)
+			return 1;
+	}
+
+	return 0;
+}
+
 int cmd_rev_list(int argc,
 		 const char **argv,
 		 const char *prefix,
@@ -789,9 +830,8 @@ int cmd_rev_list(int argc,
 	 * NEEDSWORK: The next loop is utterly broken.  It tries to
 	 * notice an option is used, but without understanding if each
 	 * option takes an argument, which fundamentally would not
-	 * work.  It would not know "--grep
-	 * --exclude-promisor-objects" is not triggering
-	 * "--exclude-promisor-objects" option, for example.
+	 * work.  It would not know that "--author --missing-only" is not
+	 * triggering "--missing-only" option, for example.
 	 *
 	 * We really need setup_revisions() to have a mechanism to
 	 * allow and disallow some sets of options for different
@@ -802,6 +842,15 @@ int cmd_rev_list(int argc,
 	 */
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
+
+		if (!strcmp(arg, "--"))
+			break;
+
+		if (rev_list_option_takes_argument(&argv[i])) {
+			i++;
+			continue;
+		}
+
 		if (!strcmp(arg, "--exclude-promisor-objects")) {
 			repo->fetch_if_missing = 0;
 			revs.exclude_promisor_objects = 1;
diff --git a/t/t6022-rev-list-missing.sh b/t/t6022-rev-list-missing.sh
index 1bd2c3bc4f..9f995968fd 100755
--- a/t/t6022-rev-list-missing.sh
+++ b/t/t6022-rev-list-missing.sh
@@ -215,6 +215,32 @@ do
 	'
 done
 
+test_expect_success "--grep accepts a pattern beginning with --" '
+	test_commit --no-tag --missing-only &&
+	commit=$(git rev-parse HEAD) &&
+	echo "$commit" >expect &&
+	git rev-list --grep=--missing-only HEAD >actual &&
+	test_cmp expect actual &&
+	git rev-list --grep --missing-only HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "--grep does not trigger --exclude-promisor-objects in early scan" '
+	test_commit --no-tag --exclude-promisor-objects &&
+	commit=$(git rev-parse HEAD) &&
+	echo "$commit" >expect &&
+	git rev-list --grep=--exclude-promisor-objects --missing=print HEAD >actual &&
+	test_cmp expect actual &&
+	git rev-list --grep --exclude-promisor-objects --missing=print HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success "--author argument is not treated as an option in early scan" '
+	test_commit --no-tag author-test &&
+	git rev-list --author --missing-only HEAD >actual &&
+	test_must_be_empty actual
+'
+
 test_expect_success "--missing-only requires --missing=print or --missing=print-info" '
 	test_must_fail git rev-list --missing-only --objects HEAD 2>err &&
 	test_grep "requires --missing=print" err
-- 
2.53.0