[PATCH 4/5] config: make numeric parsing errors more clear
- From
Jeff King <peff@peff.net>
- Date
- Sep 8, 2013, 08:38 UTC
- Message-ID
- <20130908083822.GD29554@sigill.intra.peff.net>
- In-Reply-To
- <20130908082724.GA27184@sigill.intra.peff.net>
If we try to parse an integer config argument and get a number outside of the representable range, we die with the cryptic message: "bad config value for '%s'".
We can improve two things:
1. Show the value that produced the error (e.g., bad
config value '3g' for 'foo.bar'). 2. Mention the reason the value was rejected (e.g.,
"invalid unit" versus "out of range").A few tests need to be updated with the new output, but that should not be representative of real-world breakage, as scripts should not be depending on the exact text of our stderr output, which is subject to i18n anyway.
Signed-off-by: Jeff King <peff@peff.net> --- I was tempted to clean up the tortured string construction in die_bad_number(), but my understanding is that translators prefer to see the string in as whole a piece as we can get it, rather than building it piecemeal from "if"s. And though this isn't marked for translation, it probably should be.
config.c | 17 ++++++++++++----- t/t1300-repo-config.sh | 6 +++--- t/t4055-diff-context.sh | 2 +- 3 files changed, 16 insertions(+), 9 deletions(-)
diff --git a/config.c b/config.c index 7332b06..87f87ba 100644 --- a/config.c +++ b/config.c @@ -543,18 +543,25 @@ int git_config_int(const char *name, const char *value) return 1; } -static void die_bad_config(const char *name) +static void die_bad_number(const char *name, const char *value) { + const char *reason = errno == ERANGE ? + "out of range" : + "invalid unit"; + if (!value) + value = ""; + if (cf && cf->name) - die("bad config value for '%s' in %s", name, cf->name); - die("bad config value for '%s'", name); + die("bad numeric config value '%s' for '%s' in %s: %s", + value, name, cf->name, reason); + die("bad numeric config value '%s' for '%s': %s", value, name, reason); } int git_config_int(const char *name, const char *value) { int ret; if (!git_parse_int(value, &ret)) - die_bad_config(name); + die_bad_number(name, value); return ret; } @@ -562,7 +569,7 @@ unsigned long git_config_ulong(const char *name, const char *value) { unsigned long ret; if (!git_parse_ulong(value, &ret)) - die_bad_config(name); + die_bad_number(name, value); return ret; } diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh index c4a7d84..20aee6e 100755 --- a/t/t1300-repo-config.sh +++ b/t/t1300-repo-config.sh @@ -657,11 +657,11 @@ test_expect_success 'invalid unit' ' echo 1auto >expect && git config aninvalid.unit >actual && test_cmp expect actual && - cat > expect <<-\EOF - fatal: bad config value for '\''aninvalid.unit'\'' in .git/config + cat >expect <<-\EOF + fatal: bad numeric config value '\''1auto'\'' for '\''aninvalid.unit'\'' in .git/config: invalid unit EOF test_must_fail git config --int --get aninvalid.unit 2>actual && - test_cmp actual expect + test_i18ncmp expect actual ' cat > expect << EOF diff --git a/t/t4055-diff-context.sh b/t/t4055-diff-context.sh index 97172b4..cd04543 100755 --- a/t/t4055-diff-context.sh +++ b/t/t4055-diff-context.sh @@ -73,7 +73,7 @@ test_expect_success 'non-integer config parsing' ' test_expect_success 'non-integer config parsing' ' git config diff.context no && test_must_fail git diff 2>output && - test_i18ngrep "bad config value" output + test_i18ngrep "bad numeric config value" output ' test_expect_success 'negative integer config parsing' '
-- 1.8.4.2.g87d4a77