cat-file: fix remaining usage bugs
With the migration of --batch-all-objects to OPT_CMDMODE() in the preceding commit one bug with combining it and other OPT_CMDMODE() options was solved, but we were still left with e.g. --buffer silently being discarded when not in batch mode. Fix all those bugs, and in addition emit errors telling the user specifically what options can't be combined with what other options, before this we'd usually just emit the cryptic usage text and leave the users to work it out by themselves. This change is rather large, because to do so we need to untangle the options processing so that we can not only error out, but emit sensible errors, and e.g. emit errors about options before errors about stray argc elements (as they might become valid if the option were removed). Some of the output changes ("error:" to "fatal:" with usage_msg_opt[f]()), but none of the exit codes change, except in those cases where we silently accepted bad option combinations before, now we'll error out. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ævar Arnfjörð Bjarmason committed
Dec 28, 2021 at 14:28 UTC
b3fe468075b7f0aa40f7a22a5a50e5e3e5fb2148
2 files changed
+84
-52
builtin/cat-file.c
+63
-32
@@ -648,6 +648,8 @@ static int batch_option_callback(const struct option *opt,
648
int cmd_cat_file(int argc, const char **argv, const char *prefix)
649
{
650
int opt = 0;
651
+ int opt_cw = 0;
652
+ int opt_epts = 0;
653
const char *exp_type = NULL, *obj_name = NULL;
654
struct batch_options batch = {0};
655
int unknown_type = 0;
@@ -701,45 +703,74 @@ int cmd_cat_file(int argc, const char **argv, const char *prefix)
703
batch.buffer_output = -1;
704
705
argc = parse_options(argc, argv, prefix, options, usage, 0);
704
- if (argc && batch.enabled)
705
- usage_with_options(usage, options);
706
- if (opt == 'b') {
707
- batch.all_objects = 1;
708
- } else if (opt) {
709
- if (batch.enabled && (opt == 'c' || opt == 'w'))
710
- batch.cmdmode = opt;
711
- else if (argc == 1)
712
- obj_name = argv[0];
713
- else
714
- usage_with_options(usage, options);
715
- } else if (!opt && !batch.enabled) {
716
- if (argc == 2) {
717
- exp_type = argv[0];
718
- obj_name = argv[1];
719
- } else
720
- usage_with_options(usage, options);
721
- } else if (batch.enabled && batch.cmdmode != opt)
722
- usage_with_options(usage, options);
706
+ opt_cw = (opt == 'c' || opt == 'w');
707
+ opt_epts = (opt == 'e' || opt == 'p' || opt == 't' || opt == 's');
708
724
- if ((batch.follow_symlinks || batch.all_objects) && !batch.enabled) {
725
- usage_with_options(usage, options);
726
- }
727
-
728
- if (force_path && opt != 'c' && opt != 'w') {
729
- error("--path=<path> needs --textconv or --filters");
730
- usage_with_options(usage, options);
731
- }
709
+ /* --batch-all-objects? */
710
+ if (opt == 'b')
711
+ batch.all_objects = 1;
712
733
- if (force_path && batch.enabled) {
734
- error("--path=<path> incompatible with --batch");
735
- usage_with_options(usage, options);
736
- }
713
+ /* Option compatibility */
714
+ if (force_path && !opt_cw)
715
+ usage_msg_optf(_("'%s=<%s>' needs '%s' or '%s'"),
716
+ usage, options,
717
+ "--path", _("path|tree-ish"), "--filters",
718
+ "--textconv");
719
720
+ /* Option compatibility with batch mode */
721
+ if (batch.enabled)
722
+ ;
723
+ else if (batch.follow_symlinks)
724
+ usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
725
+ "--follow_symlinks");
726
+ else if (batch.buffer_output >= 0)
727
+ usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
728
+ "--buffer");
729
+ else if (batch.all_objects)
730
+ usage_msg_optf(_("'%s' requires a batch mode"), usage, options,
731
+ "--batch-all-objects");
732
+
733
+ /* Batch defaults */
734
if (batch.buffer_output < 0)
735
batch.buffer_output = batch.all_objects;
736
741
- if (batch.enabled)
737
+ /* Return early if we're in batch mode? */
738
+ if (batch.enabled) {
739
+ if (opt_cw)
740
+ batch.cmdmode = opt;
741
+ else if (opt && opt != 'b')
742
+ usage_msg_optf(_("'-%c' is incompatible with batch mode"),
743
+ usage, options, opt);
744
+ else if (argc)
745
+ usage_msg_opt(_("batch modes take no arguments"), usage,
746
+ options);
747
+
748
return batch_objects(&batch);
749
+ }
750
+
751
+ if (opt) {
752
+ if (!argc && opt == 'c')
753
+ usage_msg_optf(_("<rev> required with '%s'"),
754
+ usage, options, "--textconv");
755
+ else if (!argc && opt == 'w')
756
+ usage_msg_optf(_("<rev> required with '%s'"),
757
+ usage, options, "--filters");
758
+ else if (!argc && opt_epts)
759
+ usage_msg_optf(_("<object> required with '-%c'"),
760
+ usage, options, opt);
761
+ else if (argc == 1)
762
+ obj_name = argv[0];
763
+ else
764
+ usage_msg_opt(_("too many arguments"), usage, options);
765
+ } else if (!argc) {
766
+ usage_with_options(usage, options);
767
+ } else if (argc != 2) {
768
+ usage_msg_optf(_("only two arguments allowed in <type> <object> mode, not %d"),
769
+ usage, options, argc);
770
+ } else if (argc) {
771
+ exp_type = argv[0];
772
+ obj_name = argv[1];
773
+ }
774
775
if (unknown_type && opt != 't' && opt != 's')
776
die("git cat-file --allow-unknown-type: use with -s or -t");
t/t1006-cat-file.sh
+21
-20
@@ -24,7 +24,7 @@ done
24
25
test_incompatible_usage () {
26
test_expect_code 129 "$@" 2>err &&
27
- grep -E "^error:.**needs" err
27
+ grep -E "^(fatal|error):.*(requires|incompatible with|needs)" err
28
}
29
30
for opt in --batch --batch-check
@@ -34,48 +34,54 @@ do
34
'
35
done
36
37
+test_missing_usage () {
38
+ test_expect_code 129 "$@" 2>err &&
39
+ grep -E "^fatal:.*required" err
40
+}
41
+
42
short_modes="-e -p -t -s"
43
cw_modes="--textconv --filters"
44
45
for opt in $cw_modes
46
do
47
test_expect_success "usage: $opt requires another option" '
43
- test_expect_code 129 git cat-file $opt
48
+ test_missing_usage git cat-file $opt
49
'
50
done
51
52
for opt in $short_modes
53
do
54
test_expect_success "usage: $opt requires another option" '
50
- test_expect_code 129 git cat-file $opt
55
+ test_missing_usage git cat-file $opt
56
'
57
58
for opt2 in --batch \
59
--batch-check \
55
- --follow-symlinks
60
+ --follow-symlinks \
61
+ "--path=foo HEAD:some-path.txt"
62
do
57
- test_expect_failure "usage: incompatible options: $opt and $opt2" '
63
+ test_expect_success "usage: incompatible options: $opt and $opt2" '
64
test_incompatible_usage git cat-file $opt $opt2
65
'
66
done
61
-
62
- opt2="--path=foo HEAD:some-path.txt"
63
- test_expect_success "usage: incompatible options: $opt and $opt2" '
64
- test_incompatible_usage git cat-file $opt $opt2
65
- '
67
done
68
69
+test_too_many_arguments () {
70
+ test_expect_code 129 "$@" 2>err &&
71
+ grep -E "^fatal: too many arguments$" err
72
+}
73
+
74
for opt in $short_modes $cw_modes
75
do
76
args="one two three"
77
test_expect_success "usage: too many arguments: $opt $args" '
72
- test_expect_code 129 git cat-file $opt $args
78
+ test_too_many_arguments git cat-file $opt $args
79
'
80
81
for opt2 in --buffer --follow-symlinks
82
do
83
test_expect_success "usage: incompatible arguments: $opt with batch option $opt2" '
78
- test_expect_code 129 git cat-file $opt $opt2
84
+ test_incompatible_usage git cat-file $opt $opt2
85
'
86
done
87
done
@@ -84,14 +90,9 @@ for opt in --buffer \
90
--follow-symlinks \
91
--batch-all-objects
92
do
87
- status=success
88
- if test $opt = "--buffer"
89
- then
90
- status=failure
91
- fi
92
- test_expect_$status "usage: bad option combination: $opt without batch mode" '
93
- test_expect_code 129 git cat-file $opt &&
94
- test_expect_code 129 git cat-file $opt commit HEAD
93
+ test_expect_success "usage: bad option combination: $opt without batch mode" '
94
+ test_incompatible_usage git cat-file $opt &&
95
+ test_incompatible_usage git cat-file $opt commit HEAD
96
'
97
done
98