config: suggest the correct form when key contains "="

* The quiet parameter now lives on a static do_parse_config_key() instead of git_config_parse_key() itself. git_config_parse_key() is back to its three-argument signature; existing callers don't change.
* New public git_config_key_is_valid() for callers that only need a yes/no check.

Harald Nordgren (2):
config: add git_config_key_is_valid() for quiet validation
config: improve diagnostic for "set" with missing value

 builtin/config.c  | 32 ++++++++++++++++++++++++++-
 config.c          | 38 ++++++++++++++++++++++++--------
 config.h          |  2 ++
 t/t1300-config.sh | 56 +++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 118 insertions(+), 10 deletions(-)

base-commit: 9ac3f193c05c2237e2b14ebaa1149e9fc8a1abe0
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2302%2FHaraldNordgren%2Fconfig-hint-equals-key-v6
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2302/HaraldNordgren/config-hint-equals-key-v6
Pull-Request: https://github.com/git/git/pull/2302

Range-diff vs v5:

1: d938ebf95a ! 1: 7400ca41bb config: let git_config_parse_key() validate quietly
@@ Metadata
Author: Harald Nordgren <[email protected]>

## Commit message ##
- config: let git_config_parse_key() validate quietly
+ config: add git_config_key_is_valid() for quiet validation

- Add a "quiet" parameter that suppresses the error() calls, and let
- store_key be NULL to skip the canonical-copy allocation. Existing
- callers pass 0 for quiet.
+ Move the body of git_config_parse_key() into a static helper
+ do_parse_config_key() that takes a "quiet" flag and treats
+ store_key as optional. git_config_parse_key() becomes a thin
+ wrapper.

- Signed-off-by: Harald Nordgren <[email protected]>
+ Add git_config_key_is_valid() for callers that only need to
+ know whether a key is well-formed.

- ## builtin/config.c ##
-@@ builtin/config.c: static int get_value(const struct config_location_options *opts,
- goto free_strings;
- }
- } else {
-- if (git_config_parse_key(key_, &key, NULL)) {
-+ if (git_config_parse_key(key_, &key, NULL, 0)) {
- ret = CONFIG_INVALID_KEY;
- goto free_strings;
- }
+ Signed-off-by: Harald Nordgren <[email protected]>

## config.c ##
@@ config.c: static inline int iskeychar(int c)
@@ config.c: static inline int iskeychar(int c)
+ * quiet - when non-zero, suppress error() reports on rejection
*/
-int git_config_parse_key(const char *key, char **store_key, size_t *baselen_)
-+int git_config_parse_key(const char *key, char **store_key, size_t *baselen_,
-+ int quiet)
++static int do_parse_config_key(const char *key, char **store_key,
++ size_t *baselen_, int quiet)
{
size_t i, baselen;
int dot;
@@ config.c: int git_config_parse_key(const char *key, char **store_key, size_t *ba
return -CONFIG_INVALID_KEY;
}

-@@ config.c: static int config_parse_pair(const char *key, const char *value, -
- if (!strlen(key))
- return error(_("empty config key"));
-- if (git_config_parse_key(key, &canonical_name, NULL))
-+ if (git_config_parse_key(key, &canonical_name, NULL, 0))
- return -1;
-
- ret = (fn(canonical_name, value, &ctx, data) < 0) ? -1 : 0;
-@@ config.c: static int configset_find_element(struct config_set *set, const char *key,
- * key may come from the user, so normalize it before using it
- * for querying entries from the hashmap.
- */
-- ret = git_config_parse_key(key, &normalized_key, NULL);
-+ ret = git_config_parse_key(key, &normalized_key, NULL, 0);
- if (ret)
- return ret;
-
-@@ config.c: int repo_config_set_multivar_in_file_gently(struct repository *r,
- validate_comment_string(comment);
-
- /* parse-key returns negative; flip the sign to feed exit(3) */
-- ret = 0 - git_config_parse_key(key, &store.key, &store.baselen);
-+ ret = 0 - git_config_parse_key(key, &store.key, &store.baselen, 0);
- if (ret)
- goto out_free;
-
++int git_config_parse_key(const char *key, char **store_key, size_t *baselen_) ++{
++ return do_parse_config_key(key, store_key, baselen_, 0);
++}
++
++int git_config_key_is_valid(const char *key)
++{
++ return !do_parse_config_key(key, NULL, NULL, 1);
++}
++
+ static int config_parse_pair(const char *key, const char *value,
+ struct key_value_info *kvi,
+ config_fn_t fn, void *data)

## config.h ##
-@@ config.h: int repo_config_set_worktree_gently(struct repository *, const char *, const cha
- */
- void repo_config_set(struct repository *, const char *, const char *);
+@@ config.h: void repo_config_set(struct repository *, const char *, const char *);

--int git_config_parse_key(const char *, char **, size_t *); -+int git_config_parse_key(const char *, char **, size_t *, int quiet);
+ int git_config_parse_key(const char *, char **, size_t *);

++int git_config_key_is_valid(const char *);
++
/*
* The following macros specify flag bits that alter the behavior
-
- ## submodule-config.c ##
-@@ submodule-config.c: int print_config_from_gitmodules(struct repository *repo, const char *key)
- int ret;
- char *store_key;
-
-- ret = git_config_parse_key(key, &store_key, NULL);
-+ ret = git_config_parse_key(key, &store_key, NULL, 0);
- if (ret < 0)
- return CONFIG_INVALID_KEY;
-
+ * of the repo_config_set_multivar*() methods.
2: e5a2070ee1 ! 2: a7f8a084c7 config: improve diagnostic for "set" with missing value

     @@ builtin/config.c: static void check_argc(int argc, int min, int max)
      +	const char *eq = last_dot ? strchr(last_dot + 1, '=') : NULL;
      +	char *prefix = eq ? xstrndup(arg, eq - arg) : NULL;
      +
     -+	if (prefix && !git_config_parse_key(prefix, NULL, NULL, 1)) {
     ++	if (prefix && git_config_key_is_valid(prefix)) {
      +		error(_("missing value to set to the variable '%s'"), arg);
      +		advise(_("did you mean \"git config set %s %s\"?"),
      +		       prefix, eq + 1);
     -+	} else if (!git_config_parse_key(arg, NULL, NULL, 1)) {
     ++	} else if (git_config_key_is_valid(arg)) {
      +		error(_("missing value to set to the variable '%s'"), arg);
      +	} else {
      +		error(_("missing value to set to a variable with an invalid name '%s'"),
     @@ t/t1300-config.sh: test_expect_success 'invalid key' '
      +
      +test_expect_success 'set with 2 args including "=" in invalid key does not suggest' '
      +	test_must_fail git config set pull.rebase=false true 2>err &&
     ++	test_grep "invalid key: pull\\.rebase=false" err &&
      +	test_grep ! "did you mean" err
      +'
      +

gitgitgadget