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>

diff --git a/builtin/config.c b/builtin/config.c
index 80aa9d8a6606b113c65798d329105cc4ae880693..3a71d3253f8a6e069c00336de1ed7f50de416164 100644 (file)
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -125,8 +125,6 @@ static const char *comment_arg;
        { OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
        PARSE_OPT_NONEG, option_parse_type, (i) }
 
-static NORETURN void usage_builtin_config(void);
-
 static int option_parse_type(const struct option *opt, const char *arg,
                             int unset)
 {
@@ -171,7 +169,7 @@ static int option_parse_type(const struct option *opt, const char *arg,
                 * --type=int'.
                 */
                error(_("only one type at a time"));
-               usage_builtin_config();
+               exit(129);
        }
        *to_type = new_type;
 
@@ -187,7 +185,7 @@ static void check_argc(int argc, int min, int max)
        else
                error(_("wrong number of arguments, should be from %d to %d"),
                      min, max);
-       usage_builtin_config();
+       exit(129);
 }
 
 static void show_config_origin(const struct key_value_info *kvi,
@@ -672,7 +670,7 @@ static void handle_config_location(const char *prefix)
            use_worktree_config +
            !!given_config_source.file + !!given_config_source.blob > 1) {
                error(_("only one config file at a time"));
-               usage_builtin_config();
+               exit(129);
        }
 
        if (!startup_info->have_repository) {
@@ -802,11 +800,6 @@ static struct option builtin_config_options[] = {
        OPT_END(),
 };
 
-static NORETURN void usage_builtin_config(void)
-{
-       usage_with_options(builtin_config_usage, builtin_config_options);
-}
-
 static int cmd_config_list(int argc, const char **argv, const char *prefix)
 {
        struct option opts[] = {
@@ -1110,7 +1103,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
        if ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {
                error(_("--get-color and variable type are incoherent"));
-               usage_builtin_config();
+               exit(129);
        }
 
        if (actions == 0)
@@ -1119,30 +1112,31 @@ int cmd_config(int argc, const char **argv, const char *prefix)
                case 2: actions = ACTION_SET; break;
                case 3: actions = ACTION_SET_ALL; break;
                default:
-                       usage_builtin_config();
+                       error(_("no action specified"));
+                       exit(129);
                }
        if (omit_values &&
            !(actions == ACTION_LIST || actions == ACTION_GET_REGEXP)) {
                error(_("--name-only is only applicable to --list or --get-regexp"));
-               usage_builtin_config();
+               exit(129);
        }
 
        if (show_origin && !(actions &
                (ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST))) {
                error(_("--show-origin is only applicable to --get, --get-all, "
                        "--get-regexp, and --list"));
-               usage_builtin_config();
+               exit(129);
        }
 
        if (default_value && !(actions & ACTION_GET)) {
                error(_("--default is only applicable to --get"));
-               usage_builtin_config();
+               exit(129);
        }
 
        if (comment_arg &&
            !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) {
                error(_("--comment is only applicable to add/set/replace operations"));
-               usage_builtin_config();
+               exit(129);
        }
 
        /* check usage of --fixed-value */
@@ -1175,7 +1169,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)
 
                if (!allowed_usage) {
                        error(_("--fixed-value only applies with 'value-pattern'"));
-                       usage_builtin_config();
+                       exit(129);
                }
 
                flags |= CONFIG_FLAGS_FIXED_VALUE;

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index f3c4d28e0600514577cf0e9922c5c1875ba17eb1..d90a69b29f870d8513edb6a73ce3ba3803815f9a 100755 (executable)
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -596,7 +596,8 @@ test_expect_success 'get bool variable with empty value' '
 
 test_expect_success 'no arguments, but no crash' '
        test_must_fail git config >output 2>&1 &&
-       test_grep usage output
+       echo "error: no action specified" >expect &&
+       test_cmp expect output
 '
 
 cat > .git/config << EOF