git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "Andrew Klotz via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Andrew Klotz <agc.klotz@gmail.com>, Andrew Klotz <agc.klotz@gmail.com>
Subject: [PATCH 1/2] config: improve error message for boolean config
Date: Fri, 18 Sep 2020 02:17:06 +0000	[thread overview]
Message-ID: <689d84672422dd96d1eb89ea6137e79ce4030248.1600395427.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.841.git.git.1600395427.gitgitgadget@gmail.com>

From: Andrew Klotz <agc.klotz@gmail.com>

Currently invalid boolean config values return messages about 'bad
numeric', which is slightly misleading when the error was due to a
boolean value. We can improve the developer experience by returning a
boolean error message when we know the value is neither a bool text or
int.

`GIT_TEST_GETTEXT_POISON` is a boolean so we no longer fail on
evaluating it as an int in `git_config_int`. Because of that we can
move the special translation case into the boolean config check where
we are now failing with an updated message

before with an invalid boolean value of `non-boolean`, its unclear what
numeric is referring to:
```
fatal: bad numeric config value 'non-boolean' for 'commit.gpgsign': invalid unit
```

now the error message mentions `non-boolean` is a bad boolean value:
```
fatal: bad boolean config value 'non-boolean' for 'commit.gpgsign'
```

Signed-off-by: Andrew Klotz <agc.klotz@gmail.com>
---
 config.c                  | 22 ++++++++++++----------
 t/t0205-gettext-poison.sh |  2 +-
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/config.c b/config.c
index 2bdff4457b..198d0d3216 100644
--- a/config.c
+++ b/config.c
@@ -996,15 +996,6 @@ static void die_bad_number(const char *name, const char *value)
 	if (!value)
 		value = "";
 
-	if (!strcmp(name, "GIT_TEST_GETTEXT_POISON"))
-		/*
-		 * We explicitly *don't* use _() here since it would
-		 * cause an infinite loop with _() needing to call
-		 * use_gettext_poison(). This is why marked up
-		 * translations with N_() above.
-		 */
-		die(bad_numeric, value, name, error_type);
-
 	if (!(cf && cf->name))
 		die(_(bad_numeric), value, name, _(error_type));
 
@@ -1097,7 +1088,18 @@ int git_config_bool_or_int(const char *name, const char *value, int *is_bool)
 		return v;
 	}
 	*is_bool = 0;
-	return git_config_int(name, value);
+	if (git_parse_int(value, &v))
+		return v;
+
+	if (!strcmp(name, "GIT_TEST_GETTEXT_POISON"))
+		/*
+		 * We explicitly *don't* use _() here since it would
+		 * cause an infinite loop with _() needing to call
+		 * use_gettext_poison().
+		 */
+		die("bad boolean config value '%s' for '%s'", value, name);
+	else
+		die(_("bad boolean config value '%s' for '%s'"), value, name);
 }
 
 int git_config_bool(const char *name, const char *value)
diff --git a/t/t0205-gettext-poison.sh b/t/t0205-gettext-poison.sh
index f9fa16ad83..b66d34c6f2 100755
--- a/t/t0205-gettext-poison.sh
+++ b/t/t0205-gettext-poison.sh
@@ -33,7 +33,7 @@ test_expect_success 'eval_gettext: our eval_gettext() fallback has poison semant
 
 test_expect_success "gettext: invalid GIT_TEST_GETTEXT_POISON value doesn't infinitely loop" "
 	test_must_fail env GIT_TEST_GETTEXT_POISON=xyz git version 2>error &&
-	grep \"fatal: bad numeric config value 'xyz' for 'GIT_TEST_GETTEXT_POISON': invalid unit\" error
+	grep \"fatal: bad boolean config value 'xyz' for 'GIT_TEST_GETTEXT_POISON'\" error
 "
 
 test_done
-- 
gitgitgadget


  reply	other threads:[~2020-09-18  2:22 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-18  2:17 [PATCH 0/2] " Andrew Klotz via GitGitGadget
2020-09-18  2:17 ` Andrew Klotz via GitGitGadget [this message]
2020-09-18 18:10   ` [PATCH 1/2] " Jeff King
2020-09-18 18:30   ` Phillip Wood
2020-09-18 19:14     ` Junio C Hamano
2020-09-18  2:17 ` [PATCH 2/2] formatting for error messages Andrew Klotz via GitGitGadget
2020-09-18 17:22 ` [PATCH 0/2] config: improve error message for boolean config Junio C Hamano
2021-02-09  1:25 ` [PATCH v2] " Andrew Klotz via GitGitGadget
2021-02-09 16:51   ` Jeff King
2021-02-09 21:02     ` Junio C Hamano
2021-02-11 20:30   ` [PATCH v3] " Andrew Klotz via GitGitGadget
2021-02-12 10:38     ` Jeff King
2021-02-12 19:52       ` Junio C Hamano
2021-01-23 23:15 [PATCH 1/2] " Andrew Klotz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=689d84672422dd96d1eb89ea6137e79ce4030248.1600395427.git.gitgitgadget@gmail.com \
    --to=gitgitgadget@gmail.com \
    --cc=agc.klotz@gmail.com \
    --cc=git@vger.kernel.org \
    --subject='Re: [PATCH 1/2] config: improve error message for boolean config' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).