[PATCH v6 0/2] config: suggest the correct form when key contains "="

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

 * 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 <haraldnordgren@gmail.com>
      
       ## 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 <haraldnordgren@gmail.com>
     +    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 <haraldnordgren@gmail.com>
      
       ## 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