diff mbox series

[06/10] cat-file: make --batch-all-objects a CMDMODE

Message ID patch-06.10-ee49e586483-20211106T214259Z-avarab@gmail.com (mailing list archive)
State Superseded
Headers show
Series cat-file: better usage UX & error messages | expand

Commit Message

Ævar Arnfjörð Bjarmason Nov. 6, 2021, 9:46 p.m. UTC
The usage of OPT_CMDMODE() in "cat-file"[1] was added in parallel with
the development of[3] the --batch-all-objects option[4], so we've
since grown[5] checks that it can't be combined with other command
modes, when it should just be made a top-level command-mode
instead. It doesn't combine with --filters, --textconv etc.

By giving parse_options() information about what options are mutually
exclusive with one another we can get the die9) message being removed
here for free, we didn't even use that removed message in some cases,
e.g. for both of:

    --batch-all-objects --textconv
    --batch-all-objects --filters

We'd take the "goto usage" in the "if (opt)" branch, and never reach
the previous message. Now we'll emit e.g.:

    $ git cat-file --batch-all-objects --filters
    error: option `filters' is incompatible with --batch-all-objects

1. b48158ac94c (cat-file: make the options mutually exclusive, 2015-05-03)
2. https://lore.kernel.org/git/xmqqtwspgusf.fsf@gitster.dls.corp.google.com/
3. https://lore.kernel.org/git/20150622104559.GG14475@peff.net/
4. 6a951937ae1 (cat-file: add --batch-all-objects option, 2015-06-22)
5. 321459439e1 (cat-file: support --textconv/--filters in batch mode, 2016-09-09)

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 builtin/cat-file.c  | 25 +++++++++++--------------
 t/t1006-cat-file.sh |  7 ++-----
 2 files changed, 13 insertions(+), 19 deletions(-)

Comments

Eric Sunshine Nov. 7, 2021, 3 a.m. UTC | #1
On Sat, Nov 6, 2021 at 5:47 PM Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:
> The usage of OPT_CMDMODE() in "cat-file"[1] was added in parallel with
> the development of[3] the --batch-all-objects option[4], so we've
> since grown[5] checks that it can't be combined with other command
> modes, when it should just be made a top-level command-mode
> instead. It doesn't combine with --filters, --textconv etc.
>
> By giving parse_options() information about what options are mutually
> exclusive with one another we can get the die9) message being removed
> here for free, we didn't even use that removed message in some cases,
> e.g. for both of:

s/die9)/die()/

>     --batch-all-objects --textconv
>     --batch-all-objects --filters
>
> We'd take the "goto usage" in the "if (opt)" branch, and never reach
> the previous message. Now we'll emit e.g.:
>
>     $ git cat-file --batch-all-objects --filters
>     error: option `filters' is incompatible with --batch-all-objects
>
> 1. b48158ac94c (cat-file: make the options mutually exclusive, 2015-05-03)
> 2. https://lore.kernel.org/git/xmqqtwspgusf.fsf@gitster.dls.corp.google.com/
> 3. https://lore.kernel.org/git/20150622104559.GG14475@peff.net/
> 4. 6a951937ae1 (cat-file: add --batch-all-objects option, 2015-06-22)
> 5. 321459439e1 (cat-file: support --textconv/--filters in batch mode, 2016-09-09)
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
diff mbox series

Patch

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 6d0f645301b..87356208134 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -674,6 +674,8 @@  int cmd_cat_file(int argc, const char **argv, const char *prefix)
 			    N_("for blob objects, run textconv on object's content"), 'c'),
 		OPT_CMDMODE(0, "filters", &opt,
 			    N_("for blob objects, run filters on object's content"), 'w'),
+		OPT_CMDMODE(0, "batch-all-objects", &opt,
+			    N_("show all objects with --batch or --batch-check"), 'b'),
 		OPT_STRING(0, "path", &force_path, N_("blob"),
 			   N_("use a specific path for --textconv/--filters")),
 		OPT_BOOL(0, "allow-unknown-type", &unknown_type,
@@ -689,8 +691,6 @@  int cmd_cat_file(int argc, const char **argv, const char *prefix)
 			batch_option_callback),
 		OPT_BOOL(0, "follow-symlinks", &batch.follow_symlinks,
 			 N_("follow in-tree symlinks (used with --batch or --batch-check)")),
-		OPT_BOOL(0, "batch-all-objects", &batch.all_objects,
-			 N_("show all objects with --batch or --batch-check")),
 		OPT_BOOL(0, "unordered", &batch.unordered,
 			 N_("do not order --batch-all-objects output")),
 		OPT_END()
@@ -699,30 +699,27 @@  int cmd_cat_file(int argc, const char **argv, const char *prefix)
 	git_config(git_cat_file_config, NULL);
 
 	batch.buffer_output = -1;
-	argc = parse_options(argc, argv, prefix, options, usage, 0);
 
-	if (opt) {
+	argc = parse_options(argc, argv, prefix, options, usage, 0);
+	if (argc && batch.enabled)
+		usage_with_options(usage, options);
+	if (opt == 'b') {
+		batch.all_objects = 1;
+	} else if (opt) {
 		if (batch.enabled && (opt == 'c' || opt == 'w'))
 			batch.cmdmode = opt;
 		else if (argc == 1)
 			obj_name = argv[0];
 		else
 			usage_with_options(usage, options);
-	}
-	if (!opt && !batch.enabled) {
+	} else if (!opt && !batch.enabled) {
 		if (argc == 2) {
 			exp_type = argv[0];
 			obj_name = argv[1];
 		} else
 			usage_with_options(usage, options);
-	}
-	if (batch.enabled) {
-		if (batch.cmdmode != opt || argc)
-			usage_with_options(usage, options);
-		if (batch.cmdmode && batch.all_objects)
-			die("--batch-all-objects cannot be combined with "
-			    "--textconv nor with --filters");
-	}
+	} else if (batch.enabled && batch.cmdmode != opt)
+		usage_with_options(usage, options);
 
 	if ((batch.follow_symlinks || batch.all_objects) && !batch.enabled) {
 		usage_with_options(usage, options);
diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh
index 0ad00e1fe73..44285f749cd 100755
--- a/t/t1006-cat-file.sh
+++ b/t/t1006-cat-file.sh
@@ -14,7 +14,8 @@  test_expect_success 'usage: cmdmode' '
 	test_cmdmode_usage git cat-file -p -t &&
 	test_cmdmode_usage git cat-file -t -s &&
 	test_cmdmode_usage git cat-file -s --textconv &&
-	test_cmdmode_usage git cat-file --textconv --filters
+	test_cmdmode_usage git cat-file --textconv --filters &&
+	test_cmdmode_usage git cat-file --batch-all-objects -e
 '
 
 test_incompatible_usage() {
@@ -37,10 +38,6 @@  do
 	test_expect_success "usage: $opt requires another option" '
 		test_expect_code 129 git cat-file $opt
 	'
-
-	test_expect_failure "usage: incompatible options: --batch-all-objects with $opt" '
-		test_incompatible_usage git cat-file --batch-all-objects $opt
-	'
 done
 
 for opt in $short_modes