Thread (25 messages) flat view 25 messages, 5 authors, 2016-06-15

Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value

From: Ramsay Jones <hidden>
Date: 2016-06-15 23:08:16
Subsystem: the rest · Maintainer: Linus Torvalds


On 16/02/16 17:38, Jeff King wrote:
On Tue, Feb 16, 2016 at 04:46:07PM +0000, Ramsay Jones wrote:
quoted
quoted
OK, I am happy to add the extra code. 
Unless I've missed something (quite possible), this patch does not
need to change. (you have (both) convinced me that your current
solution is the best).

The only change that I would suggest is the one-liner I already
suggested to the previous patch (plus the one-liner in the test,
of course. err ... so the two-liner!). Having said that, I didn't
try it out - I was just typing into my email client, so ...
I think it's more than that one-liner. This patch shows "type:name"
verbatim from what is passed into do_config_from_file, as does the error
message. If they are going to have different output formats (e.g.,
"<stdin>" versus "stdin"), there needs to be logic transforming them in
at least one of the spots.
Ugh, yes you are right.

Hmm, I just hacked something up (see below) and, since its a bit
ugly, I'm now in two minds! (it could be improved, of course). ;-)

So, I'll leave it to yourself and Lars to decide.

ATB,
Ramsay Jones

-- >8 --
From: Ramsay Jones <redacted>
Date: Tue, 16 Feb 2016 21:39:47 +0000
Subject: [PATCH] config: fixup '<stdin>' error messages

Signed-off-by: Ramsay Jones <redacted>
---
 config.c               | 22 ++++++++++++++++++----
 t/t1300-repo-config.sh |  2 +-
 2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/config.c b/config.c
index 0a35323..adc808c 100644
--- a/config.c
+++ b/config.c
@@ -417,6 +417,8 @@ static int git_parse_source(config_fn_t fn, void *data)
 	int comment = 0;
 	int baselen = 0;
 	struct strbuf *var = &cf->var;
+	const char *cftype = cf->type;
+	const char *cfname = cf->name;
 
 	/* U+FEFF Byte Order Mark in UTF8 */
 	const char *bomptr = utf8_bom;
@@ -471,10 +473,14 @@ static int git_parse_source(config_fn_t fn, void *data)
 		if (get_value(fn, data, var) < 0)
 			break;
 	}
+	if (!strcmp(cftype, "stdin")) {
+		cftype = "file";
+		cfname = "<stdin>";
+	}
 	if (cf->die_on_error)
-		die(_("bad config line %d in %s %s"), cf->linenr, cf->type, cf->name);
+		die(_("bad config line %d in %s %s"), cf->linenr, cftype, cfname);
 	else
-		return error(_("bad config line %d in %s %s"), cf->linenr, cf->type, cf->name);
+		return error(_("bad config line %d in %s %s"), cf->linenr, cftype, cfname);
 }
 
 static int parse_unit_factor(const char *end, uintmax_t *val)
@@ -589,9 +595,17 @@ static void die_bad_number(const char *name, const char *value)
 	if (!value)
 		value = "";
 
-	if (cf && cf->type && cf->name)
+	if (cf && cf->type && cf->name) {
+		const char *cftype = cf->type;
+		const char *cfname = cf->name;
+
+		if (!strcmp(cftype, "stdin")) {
+			cftype = "file";
+			cfname = "<stdin>";
+		}
 		die(_("bad numeric config value '%s' for '%s' in %s %s: %s"),
-		    value, name, cf->type, cf->name, reason);
+		    value, name, cftype, cfname, reason);
+	}
 	die(_("bad numeric config value '%s' for '%s': %s"), value, name, reason);
 }
 
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 4abbdf9..4497688 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -707,7 +707,7 @@ test_expect_success 'invalid unit' '
 '
 
 test_expect_success 'invalid stdin config' '
-	echo "fatal: bad config line 1 in stdin " >expect &&
+	echo "fatal: bad config line 1 in file <stdin>" >expect &&
 	echo "[broken" | test_must_fail git config --list --file - >output 2>&1 &&
 	test_cmp expect output
 '
-- 
2.7.0
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help