diff mbox series

[v10,12/12] merge-index: make the argument parsing sensible & simpler

Message ID patch-v10-12.12-40b6d296f3a-20221215T084803Z-avarab@gmail.com (mailing list archive)
State New, archived
Headers show
Series merge-index: prepare to rewrite merge drivers in C | expand

Commit Message

Ævar Arnfjörð Bjarmason Dec. 15, 2022, 8:52 a.m. UTC
In a preceding commit when we migrated to parse_options() we took
pains to be bug-for-bug compatible with the existing command-line
interface, if possible.

I.e. we forbade forms like:

	git merge-index -a <program>
	git merge-index <program> <opts> -a

But allowed:

	git merge-index <program> -a
	git merge-index <opts> <program> -a

As the "-a" argument was considered be provided for the "<program>",
but not a part of "<opts>".

We don't really need this strictness, as we don't have two "-a"
options. It's much simpler to implement a schema where the first
non-option argument is the <program>, and the rest are the
"<file>...". We only allow that rest if the "-a" option isn't
supplied.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/merge-index.c  | 28 ++++++++--------------------
 t/t6060-merge-index.sh | 12 +++++++++---
 2 files changed, 17 insertions(+), 23 deletions(-)
diff mbox series

Patch

diff --git a/builtin/merge-index.c b/builtin/merge-index.c
index d679272391b..d8b62e4f663 100644
--- a/builtin/merge-index.c
+++ b/builtin/merge-index.c
@@ -59,21 +59,14 @@  int cmd_merge_index(int argc, const char **argv, const char *prefix)
 		N_("git merge-index [-o] [-q] <merge-program> (-a | ([--] <file>...))"),
 		NULL
 	};
-#define OPT__MERGE_INDEX_ALL(v) \
-	OPT_BOOL('a', NULL, (v), \
-		 N_("merge all files in the index that need merging"))
 	struct option options[] = {
 		OPT_BOOL('o', NULL, &one_shot,
 			 N_("don't stop at the first failed merge")),
 		OPT__QUIET(&quiet, N_("be quiet")),
-		OPT__MERGE_INDEX_ALL(&all), /* include "-a" to show it in "-bh" */
+		OPT_BOOL('a', NULL, &all,
+			 N_("merge all files in the index that need merging")),
 		OPT_END(),
 	};
-	struct option options_prog[] = {
-		OPT__MERGE_INDEX_ALL(&all),
-		OPT_END(),
-	};
-#undef OPT__MERGE_INDEX_ALL
 	struct mofs_data data = { 0 };
 
 	/* Without this we cannot rely on waitpid() to tell
@@ -81,20 +74,15 @@  int cmd_merge_index(int argc, const char **argv, const char *prefix)
 	 */
 	signal(SIGCHLD, SIG_DFL);
 
-	if (argc < 3)
-		usage_with_options(usage, options);
-
-	/* Option parsing without <merge-program> options */
-	argc = parse_options(argc, argv, prefix, options, usage,
-			     PARSE_OPT_STOP_AT_NON_OPTION);
-	if (all)
-		usage_msg_optf(_("'%s' option can only be provided after '<merge-program>'"),
-			      usage, options, "-a");
-	/* <merge-program> and its options */
+	argc = parse_options(argc, argv, prefix, options, usage, 0);
 	if (!argc)
 		usage_msg_opt(_("need a <merge-program> argument"), usage, options);
 	data.program = argv[0];
-	argc = parse_options(argc, argv, prefix, options_prog, usage, 0);
+	argv++;
+	argc--;
+	if (!argc && !all)
+		usage_msg_opt(_("need '-a' or '<file>...'"),
+			      usage, options);
 	if (argc && all)
 		usage_msg_opt(_("'-a' and '<file>...' are mutually exclusive"),
 			      usage, options);
diff --git a/t/t6060-merge-index.sh b/t/t6060-merge-index.sh
index bc201a69552..4ff9ace7f73 100755
--- a/t/t6060-merge-index.sh
+++ b/t/t6060-merge-index.sh
@@ -22,7 +22,7 @@  test_expect_success 'usage: 2 arguments' '
 
 test_expect_success 'usage: -a before <program>' '
 	cat >expect <<-\EOF &&
-	fatal: '\''-a'\'' option can only be provided after '\''<merge-program>'\''
+	fatal: '\''-a'\'' and '\''<file>...'\'' are mutually exclusive
 	EOF
 	test_expect_code 129 git merge-index -a b program >out 2>actual.raw &&
 	grep "^fatal:" actual.raw >actual &&
@@ -34,7 +34,7 @@  for opt in -q -o
 do
 	test_expect_success "usage: $opt after -a" '
 		cat >expect <<-EOF &&
-		fatal: '\''-a'\'' option can only be provided after '\''<merge-program>'\''
+		fatal: need a <merge-program> argument
 		EOF
 		test_expect_code 129 git merge-index -a $opt >out 2>actual.raw &&
 		grep "^fatal:" actual.raw >actual &&
@@ -43,7 +43,13 @@  do
 	'
 
 	test_expect_success "usage: $opt program" '
-		test_expect_code 0 git merge-index $opt program
+		cat >expect <<-EOF &&
+		fatal: need '\''-a'\'' or '\''<file>...'\''
+		EOF
+		test_expect_code 129 git merge-index $opt program 2>actual.raw &&
+		grep "^fatal:" actual.raw >actual &&
+		test_must_be_empty out &&
+		test_cmp expect actual
 	'
 done