config: suggest the correct form when key contains "=" in set context

A user who types "git config pull.rebase=false" gets only "error:
invalid key: pull.rebase=false" with no clue what went wrong.

Emit a "did you mean ..." hint suggesting the split form.  Restrict it
to plausible-set contexts ("git config set", bare "git config <key>",
and their 2-arg forms); explicit "get"/"unset" keep the existing error.

"=" is legal inside a subsection, so only fire when "=" lands after
the last ".".  When the user supplied a separate value, use it in the
suggestion instead of the suffix after "=":

    $ git config set pull.rebase=false true
    error: invalid key: pull.rebase=false
    hint: did you mean "git config set pull.rebase true"?

Signed-off-by: Harald Nordgren <harald.nordgren@kostdoktorn.se>
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/config.c b/builtin/config.c
index cf4ba0f7cc6f228b16c1f131619837f7a8d441c0..8c7ab36fcbed8ab8533031d1ea032a069c8fde39 100644 (file)
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -1,6 +1,7 @@
 #define USE_THE_REPOSITORY_VARIABLE
 #include "builtin.h"
 #include "abspath.h"
+#include "advice.h"
 #include "config.h"
 #include "color.h"
 #include "date.h"
@@ -210,6 +211,24 @@ static void check_argc(int argc, int min, int max)
        exit(129);
 }
 
+static void advise_setting_with_equals(const char *key, const char *value)
+{
+       const char *last_dot = strrchr(key, '.');
+       const char *eq;
+
+       if (!last_dot)
+               return;
+       eq = strchr(last_dot + 1, '=');
+       if (!eq)
+               return;
+       if (!value)
+               value = eq + 1;
+       if (!*value || strpbrk(value, " \t\n"))
+               return;
+       advise(_("did you mean \"git config set %.*s %s\"?"),
+              (int)(eq - key), key, value);
+}
+
 static void show_config_origin(const struct config_display_options *opts,
                               const struct key_value_info *kvi,
                               struct strbuf *buf)
@@ -1133,6 +1152,11 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
 
        argc = parse_options(argc, argv, prefix, opts, builtin_config_set_usage,
                             PARSE_OPT_STOP_AT_NON_OPTION);
+       if (argc == 1 && strchr(argv[0], '=')) {
+               error(_("wrong number of arguments, should be 2"));
+               advise_setting_with_equals(argv[0], NULL);
+               exit(129);
+       }
        check_argc(argc, 2, 2);
 
        if ((flags & CONFIG_FLAGS_FIXED_VALUE) && !value_pattern)
@@ -1160,6 +1184,8 @@ static int cmd_config_set(int argc, const char **argv, const char *prefix,
                        error(_("cannot overwrite multiple values with a single value\n"
                        "       Use --value=<pattern>, --append or --all to change %s."), argv[0]);
        }
+       if (ret == CONFIG_INVALID_KEY)
+               advise_setting_with_equals(argv[0], argv[1]);
 
        location_options_release(&location_opts);
        free(comment);
@@ -1371,6 +1397,7 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
        };
        char *value = NULL, *comment = NULL;
        int ret = 0;
+       int actions_implicit;
        struct key_value_info default_kvi = KVI_INIT;
 
        argc = parse_options(argc, argv, prefix, opts,
@@ -1385,7 +1412,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
                exit(129);
        }
 
-       if (actions == 0)
+       actions_implicit = (actions == 0);
+       if (actions_implicit)
                switch (argc) {
                case 1: actions = ACTION_GET; break;
                case 2: actions = ACTION_SET; break;
@@ -1485,6 +1513,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
                if (ret == CONFIG_NOTHING_SET)
                        error(_("cannot overwrite multiple values with a single value\n"
                        "       Use a regexp, --add or --replace-all to change %s."), argv[0]);
+               else if (ret == CONFIG_INVALID_KEY)
+                       advise_setting_with_equals(argv[0], argv[1]);
        }
        else if (actions == ACTION_SET_ALL) {
                check_write(&location_opts.source);
@@ -1515,6 +1545,8 @@ static int cmd_config_actions(int argc, const char **argv, const char *prefix)
                check_argc(argc, 1, 2);
                ret = get_value(&location_opts, &display_opts, argv[0], argv[1],
                                0, flags);
+               if (ret == CONFIG_INVALID_KEY && actions_implicit)
+                       advise_setting_with_equals(argv[0], NULL);
        }
        else if (actions == ACTION_GET_ALL) {
                check_argc(argc, 1, 2);

diff --git a/t/t1300-config.sh b/t/t1300-config.sh
index 128971ee12fa6c510c4110d9b958037fbf578f42..44040f38b95a4b7530774a5ec5e77042c8740717 100755 (executable)
--- a/t/t1300-config.sh
+++ b/t/t1300-config.sh
@@ -462,6 +462,59 @@ test_expect_success 'invalid key' '
        test_must_fail git config inval.2key blabla
 '
 
+test_expect_success 'misplaced "=" in key: bare 1-arg form hints' '
+       test_must_fail git config pull.rebase=false 2>err &&
+       test_grep "invalid key: pull\\.rebase=false" err &&
+       test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'misplaced "=" in key: bare 2-arg form uses given value' '
+       test_must_fail git config pull.rebase=false true 2>err &&
+       test_grep "did you mean .git config set pull\\.rebase true." err
+'
+
+test_expect_success 'misplaced "=" in key: set subcommand uses given value' '
+       test_must_fail git config set pull.rebase=false true 2>err &&
+       test_grep "did you mean .git config set pull\\.rebase true." err
+'
+
+test_expect_success 'misplaced "=" in key: set with single arg hints' '
+       test_must_fail git config set pull.rebase=false 2>err &&
+       test_grep "wrong number of arguments" err &&
+       test_grep "did you mean .git config set pull\\.rebase false." err
+'
+
+test_expect_success 'misplaced "=" in key: explicit --get does not hint' '
+       test_must_fail git config --get pull.rebase=false 2>err &&
+       test_grep "invalid key: pull\\.rebase=false" err &&
+       test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: get subcommand does not hint' '
+       test_must_fail git config get pull.rebase=false 2>err &&
+       test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: unset subcommand does not hint' '
+       test_must_fail git config unset pull.rebase=false 2>err &&
+       test_grep ! "did you mean" err
+'
+
+test_expect_success 'misplaced "=" in key: value with whitespace skips hint' '
+       test_must_fail git config set pull.rebase=false "hello world" 2>err &&
+       test_grep "invalid key: pull\\.rebase=false" err &&
+       test_grep ! "did you mean" err
+'
+
+test_expect_success '"=" inside subsection is valid, no hint' '
+       test_when_finished "rm -f subsection.cfg" &&
+       git config set -f subsection.cfg foo.bar=baz.boo qux 2>err &&
+       test_grep ! "did you mean" err &&
+       echo qux >expect &&
+       git config get -f subsection.cfg foo.bar=baz.boo >actual &&
+       test_cmp expect actual
+'
+
 test_expect_success 'correct key' '
        git config 123456.a123 987
 '