builtin/config: stop printing full usage on misuse

When invoking git-config(1) with a wrong set of arguments we end up calling `usage_builtin_config()` after printing an error message that says what was wrong. As that function ends up printing the full list of options, which is quite long, the actual error message will be buried by a wall of text. This makes it really hard to figure out what exactly caused the error. Furthermore, now that we have recently introduced subcommands, the usage information may actually be misleading as we unconditionally print options of the subcommand-less mode. Fix both of these issues by just not printing the options at all anymore. Instead, we call `usage()` that makes us report in a single line what has gone wrong. This should be way more discoverable for our users and addresses the inconsistency. Furthermore, this change allow us to inline the options into the respective functions that use them to parse the command line. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed May 15, 2024 at 08:41 UTC a577d2f1a93ecfafbedd5a30cc15e4180253f04d
2 files changed +13 -18
builtin/config.c
+11 -17
@@ -125,8 +125,6 @@ static const char *comment_arg;
125 { OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
126 PARSE_OPT_NONEG, option_parse_type, (i) }
127
128 -static NORETURN void usage_builtin_config(void);
129 -
128 static int option_parse_type(const struct option *opt, const char *arg,
129 int unset)
130 {
@@ -171,7 +169,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
169 * --type=int'.
170 */
171 error(_("only one type at a time"));
174 - usage_builtin_config();
172 + exit(129);
173 }
174 *to_type = new_type;
175
@@ -187,7 +185,7 @@ static void check_argc(int argc, int min, int max)
185 else
186 error(_("wrong number of arguments, should be from %d to %d"),
187 min, max);
190 - usage_builtin_config();
188 + exit(129);
189 }
190
191 static void show_config_origin(const struct key_value_info *kvi,
@@ -672,7 +670,7 @@ static void handle_config_location(const char *prefix)
670 use_worktree_config +
671 !!given_config_source.file + !!given_config_source.blob > 1) {
672 error(_("only one config file at a time"));
675 - usage_builtin_config();
673 + exit(129);
674 }
675
676 if (!startup_info->have_repository) {
@@ -802,11 +800,6 @@ static struct option builtin_config_options[] = {
800 OPT_END(),
801 };
802
805 -static NORETURN void usage_builtin_config(void)
806 -{
807 - usage_with_options(builtin_config_usage, builtin_config_options);
808 -}
809 -
803 static int cmd_config_list(int argc, const char **argv, const char *prefix)
804 {
805 struct option opts[] = {
@@ -1110,7 +1103,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
1103
1104 if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {
1105 error(_("--get-color and variable type are incoherent"));
1113 - usage_builtin_config();
1106 + exit(129);
1107 }
1108
1109 if (actions == 0)
@@ -1119,30 +1112,31 @@ int cmd_config(int argc, const char **argv, const char *prefix)
1112 case 2: actions = ACTION_SET; break;
1113 case 3: actions = ACTION_SET_ALL; break;
1114 default:
1122 - usage_builtin_config();
1115 + error(_("no action specified"));
1116 + exit(129);
1117 }
1118 if (omit_values &&
1119 !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
1120 error(_("--name-only is only applicable to --list or --get-regexp"));
1127 - usage_builtin_config();
1121 + exit(129);
1122 }
1123
1124 if (show_origin && !(actions &
1125 (ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST))) {
1126 error(_("--show-origin is only applicable to --get, --get-all, "
1127 "--get-regexp, and --list"));
1134 - usage_builtin_config();
1128 + exit(129);
1129 }
1130
1131 if (default_value && !(actions & ACTION_GET)) {
1132 error(_("--default is only applicable to --get"));
1139 - usage_builtin_config();
1133 + exit(129);
1134 }
1135
1136 if (comment_arg &&
1137 !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
1138 error(_("--comment is only applicable to add/set/replace operations"));
1145 - usage_builtin_config();
1139 + exit(129);
1140 }
1141
1142 /* check usage of --fixed-value */
@@ -1175,7 +1169,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
1169
1170 if (!allowed_usage) {
1171 error(_("--fixed-value only applies with 'value-pattern'"));
1178 - usage_builtin_config();
1172 + exit(129);
1173 }
1174
1175 flags |= CONFIG_FLAGS_FIXED_VALUE;
t/t1300-config.sh
+2 -1
@@ -596,7 +596,8 @@ test_expect_success 'get bool variable with empty value' '
596
597 test_expect_success 'no arguments, but no crash' '
598 test_must_fail git config >output 2>&1 &&
599 - test_grep usage output
599 + echo "error: no action specified" >expect &&
600 + test_cmp expect output
601 '
602
603 cat > .git/config << EOF