{"thread":{"id":"41413","subject":"[PATCH v4 0/3] config: add '--sources' option to print the source of a config value","startedAt":"2016-02-15T10:17:43Z","lastAt":"2016-02-18T08:46:36Z","messageCount":32,"participants":["larsxschneider@gmail.com","Jeff King","Eric Sunshine","Ramsay Jones","Junio C Hamano","Lars Schneider","Johannes Schindelin"],"isPatch":true,"patchVersion":4,"patchTotal":3},"messages":[{"id":"278153","messageId":"1455531466-16617-1-git-send-email-larsxschneider@gmail.com","threadId":"41413","inReplyTo":null,"subject":"[PATCH v4 0/3] config: add '--sources' option to print the source of a config value","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-15T10:17:43Z","receivedAt":"2016-02-15T10:17:43Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\ndiff to v3:\n\n* pass type as parameter to \"git_config_from_mem\" (renamed from\n \"git_config_from_buf\") and \"do_config_from_file\"\n* split current_config_type_name into two functions\n* explain usage of fwrite with a comment\n* use tabs instead of spaces to fix indentation error\n* squash Peff's commit to split the test cases\n* sort all test configs key/value pairs in the same way\n* add comment to explain newline introduce by here-doc\n* use real tabs for test file name\n* add a test teardown\n* fix hiding of git exit code t1300 and t7008\n* drop \"git-config.txt: describe '--includes' default behavior\" patch\n\nI like the idea of a \"test set up block\" within a test script. In order\nto clean up nicely before any subsequent tests I would like to propose\na \"tear down\" block. Would that work as a compromise in our \"test cases\ndepend on earlier test cases\" discussion?\n\nIn \"t: do not hide Git's exit code in tests\" I also fixed a few more\nplaces where Git's exit code was hidden. Please drop this patch if you\nthink that this should not be part of this series.\n\nThanks a lot for the reviews and explainations,\nLars\n\nLars Schneider (3):\n t: do not hide Git's exit code in tests\n config: add 'type' to config_source struct that identifies config type\n config: add '--show-origin' option to print the origin of a config\n value\n\n Documentation/git-config.txt | 15 ++--\n builtin/config.c | 33 +++++++++\n cache.h | 6 +-\n config.c | 36 +++++++---\n submodule-config.c | 4 +-\n t/t1300-repo-config.sh | 167 ++++++++++++++++++++++++++++++++++++++++++-\n t/t1308-config-set.sh | 4 +-\n t/t7008-grep-binary.sh | 3 +-\n 8 files changed, 242 insertions(+), 26 deletions(-)\n\n--\n2.5.1\n"},{"id":"278155","messageId":"1455531466-16617-2-git-send-email-larsxschneider@gmail.com","threadId":"41413","inReplyTo":"1455531466-16617-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 1/3] t: do not hide Git's exit code in tests","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-15T10:17:44Z","receivedAt":"2016-02-15T10:17:44Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nGit should not be on the left-hand side of a pipe, because it hides the exit\ncode, and we want to make sure git does not fail.\n\nThere is one more occurrence of this pattern in t9010-svn-fe.sh which is to\nevolved to change it easily.\n\nHelped-by: Jeff King \nSigned-off-by: Lars Schneider \n---\n t/t1300-repo-config.sh | 6 ++++--\n t/t7008-grep-binary.sh | 3 ++-\n 2 files changed, 6 insertions(+), 3 deletions(-)\n\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 52678e7..1782add 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -957,13 +957,15 @@ Qsection.sub=section.val4\n Qsection.sub=section.val5Q\n EOF\n test_expect_success '--null --list' '\n-\tgit config --null --list | nul_to_q >result &&\n+\tgit config --null --list >result.raw &&\n+\tnul_to_q result &&\n \techo >>result &&\n \ttest_cmp expect result\n '\n \n test_expect_success '--null --get-regexp' '\n-\tgit config --null --get-regexp \"val[0-9]\" | nul_to_q >result &&\n+\tgit config --null --get-regexp \"val[0-9]\" >result.raw &&\n+\tnul_to_q result &&\n \techo >>result &&\n \ttest_cmp expect result\n '\ndiff --git a/t/t7008-grep-binary.sh b/t/t7008-grep-binary.sh\nindex b146406..9c9c378 100755\n--- a/t/t7008-grep-binary.sh\n+++ b/t/t7008-grep-binary.sh\n@@ -141,7 +141,8 @@ test_expect_success 'grep respects not-binary diff attribute' '\n \ttest_cmp expect actual &&\n \techo \"b diff\" >.gitattributes &&\n \techo \"b:binQary\" >expect &&\n-\tgit grep bin b | nul_to_q >actual &&\n+\tgit grep bin b >actual.raw &&\n+\tnul_to_q actual &&\n \ttest_cmp expect actual\n '\n \n-- \n2.5.1\n"},{"id":"278156","messageId":"1455531466-16617-3-git-send-email-larsxschneider@gmail.com","threadId":"41413","inReplyTo":"1455531466-16617-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-15T10:17:45Z","receivedAt":"2016-02-15T10:17:45Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nUse the config type to print more detailed error messages that inform\nthe user about the origin of a config error (file, stdin, blob).\n\nSigned-off-by: Lars Schneider \n---\n cache.h | 6 ++++--\n config.c | 36 +++++++++++++++++++++++++-----------\n submodule-config.c | 4 ++--\n t/t1300-repo-config.sh | 8 +++++++-\n t/t1308-config-set.sh | 4 ++--\n 5 files changed, 40 insertions(+), 18 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex c63fcc1..8d86e5c 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1485,8 +1485,8 @@ struct git_config_source {\n typedef int (*config_fn_t)(const char *, const char *, void *);\n extern int git_default_config(const char *, const char *, void *);\n extern int git_config_from_file(config_fn_t fn, const char *, void *);\n-extern int git_config_from_buf(config_fn_t fn, const char *name,\n-\t\t\t const char *buf, size_t len, void *data);\n+extern int git_config_from_mem(config_fn_t fn, const char *type,\n+\t\t\t\t\tconst char *name, const char *buf, size_t len, void *data);\n extern void git_config_push_parameter(const char *text);\n extern int git_config_from_parameters(config_fn_t fn, void *data);\n extern void git_config(config_fn_t fn, void *);\n@@ -1525,6 +1525,8 @@ extern const char *get_log_output_encoding(void);\n extern const char *get_commit_output_encoding(void);\n \n extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n+extern const char *current_config_type();\n+extern const char *current_config_name();\n \n struct config_include_data {\n \tint depth;\ndiff --git a/config.c b/config.c\nindex 86a5eb2..5cf11b5 100644\n--- a/config.c\n+++ b/config.c\n@@ -24,6 +24,7 @@ struct config_source {\n \t\t\tsize_t pos;\n \t\t} buf;\n \t} u;\n+\tconst char *type;\n \tconst char *name;\n \tconst char *path;\n \tint die_on_error;\n@@ -471,9 +472,9 @@ static int git_parse_source(config_fn_t fn, void *data)\n \t\t\tbreak;\n \t}\n \tif (cf->die_on_error)\n-\t\tdie(_(\"bad config file line %d in %s\"), cf->linenr, cf->name);\n+\t\tdie(_(\"bad config line %d in %s %s\"), cf->linenr, cf->type, cf->name);\n \telse\n-\t\treturn error(_(\"bad config file line %d in %s\"), cf->linenr, cf->name);\n+\t\treturn error(_(\"bad config line %d in %s %s\"), cf->linenr, cf->type, cf->name);\n }\n \n static int parse_unit_factor(const char *end, uintmax_t *val)\n@@ -588,9 +589,9 @@ static void die_bad_number(const char *name, const char *value)\n \tif (!value)\n \t\tvalue = \"\";\n \n-\tif (cf && cf->name)\n-\t\tdie(_(\"bad numeric config value '%s' for '%s' in %s: %s\"),\n-\t\t value, name, cf->name, reason);\n+\tif (cf && cf->type && cf->name)\n+\t\tdie(_(\"bad numeric config value '%s' for '%s' in %s %s: %s\"),\n+\t\t value, name, cf->type, cf->name, reason);\n \tdie(_(\"bad numeric config value '%s' for '%s': %s\"), value, name, reason);\n }\n \n@@ -1061,11 +1062,13 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)\n }\n \n static int do_config_from_file(config_fn_t fn,\n-\t\tconst char *name, const char *path, FILE *f, void *data)\n+\t\tconst char *type, const char *name, const char *path, FILE *f,\n+\t\tvoid *data)\n {\n \tstruct config_source top;\n \n \ttop.u.file = f;\n+\ttop.type = type;\n \ttop.name = name;\n \ttop.path = path;\n \ttop.die_on_error = 1;\n@@ -1078,7 +1081,7 @@ static int do_config_from_file(config_fn_t fn,\n \n static int git_config_from_stdin(config_fn_t fn, void *data)\n {\n-\treturn do_config_from_file(fn, \"\", NULL, stdin, data);\n+\treturn do_config_from_file(fn, \"stdin\", \"\", NULL, stdin, data);\n }\n \n int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n@@ -1089,21 +1092,22 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n \tf = fopen(filename, \"r\");\n \tif (f) {\n \t\tflockfile(f);\n-\t\tret = do_config_from_file(fn, filename, filename, f, data);\n+\t\tret = do_config_from_file(fn, \"file\", filename, filename, f, data);\n \t\tfunlockfile(f);\n \t\tfclose(f);\n \t}\n \treturn ret;\n }\n \n-int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,\n-\t\t\tsize_t len, void *data)\n+int git_config_from_mem(config_fn_t fn, const char *type, const char *name,\n+\t\t\tconst char *buf, size_t len, void *data)\n {\n \tstruct config_source top;\n \n \ttop.u.buf.buf = buf;\n \ttop.u.buf.len = len;\n \ttop.u.buf.pos = 0;\n+\ttop.type = type;\n \ttop.name = name;\n \ttop.path = NULL;\n \ttop.die_on_error = 0;\n@@ -1132,7 +1136,7 @@ static int git_config_from_blob_sha1(config_fn_t fn,\n \t\treturn error(\"reference '%s' does not point to a blob\", name);\n \t}\n \n-\tret = git_config_from_buf(fn, name, buf, size, data);\n+\tret = git_config_from_mem(fn, \"blob\", name, buf, size, data);\n \tfree(buf);\n \n \treturn ret;\n@@ -2385,3 +2389,13 @@ int parse_config_key(const char *var,\n \n \treturn 0;\n }\n+\n+const char *current_config_type()\n+{\n+\treturn cf && cf->type ? cf->type : \"cmdline\";\n+}\n+\n+const char *current_config_name()\n+{\n+\treturn cf && cf->name ? cf->name : \"\";\n+}\ndiff --git a/submodule-config.c b/submodule-config.c\nindex fe8ceab..92502b5 100644\n--- a/submodule-config.c\n+++ b/submodule-config.c\n@@ -427,8 +427,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,\n \tparameter.commit_sha1 = commit_sha1;\n \tparameter.gitmodules_sha1 = sha1;\n \tparameter.overwrite = 0;\n-\tgit_config_from_buf(parse_config, rev.buf, config, config_size,\n-\t\t\t¶meter);\n+\tgit_config_from_mem(parse_config, \"submodule-blob\", rev.buf,\n+\t\t\tconfig, config_size, ¶meter);\n \tfree(config);\n \n \tswitch (lookup_type) {\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 1782add..42ed5cc 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -700,12 +700,18 @@ test_expect_success 'invalid unit' '\n \tgit config aninvalid.unit >actual &&\n \ttest_cmp expect actual &&\n \tcat >expect <<-\\EOF &&\n-\tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'' in .git/config: invalid unit\n+\tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'' in file .git/config: invalid unit\n \tEOF\n \ttest_must_fail git config --int --get aninvalid.unit 2>actual &&\n \ttest_i18ncmp expect actual\n '\n \n+test_expect_success 'invalid stdin config' '\n+\techo \"fatal: bad config line 1 in stdin \" >expect &&\n+\techo \"[broken\" | test_must_fail git config --list --file - >output 2>&1 &&\n+\ttest_cmp expect output\n+'\n+\n cat > expect << EOF\n true\n false\ndiff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\nindex 91235b7..82f82a1 100755\n--- a/t/t1308-config-set.sh\n+++ b/t/t1308-config-set.sh\n@@ -195,14 +195,14 @@ test_expect_success 'proper error on error in default config files' '\n \tcp .git/config .git/config.old &&\n \ttest_when_finished \"mv .git/config.old .git/config\" &&\n \techo \"[\" >>.git/config &&\n-\techo \"fatal: bad config file line 34 in .git/config\" >expect &&\n+\techo \"fatal: bad config line 34 in file .git/config\" >expect &&\n \ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'proper error on error in custom config files' '\n \techo \"[\" >>syntax-error &&\n-\techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n+\techo \"fatal: bad config line 1 in file syntax-error\" >expect &&\n \ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n \ttest_cmp expect actual\n '\n-- \n2.5.1\n"},{"id":"278154","messageId":"1455531466-16617-4-git-send-email-larsxschneider@gmail.com","threadId":"41413","inReplyTo":"1455531466-16617-1-git-send-email-larsxschneider@gmail.com","subject":"[PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-15T10:17:46Z","receivedAt":"2016-02-15T10:17:46Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"From: Lars Schneider \n\nIf config values are queried using 'git config' (e.g. via --get,\n--get-all, --get-regexp, or --list flag) then it is sometimes hard to\nfind the configuration file where the values were defined.\n\nTeach 'git config' the '--show-origin' option to print the source\nconfiguration file for every printed value.\n\nBased-on-patch-by: Jeff King \nSigned-off-by: Lars Schneider \n---\n Documentation/git-config.txt | 15 +++--\n builtin/config.c | 33 ++++++++++\n t/t1300-repo-config.sh | 153 +++++++++++++++++++++++++++++++++++++++++++\n 3 files changed, 196 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex 2608ca7..6374997 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -9,18 +9,18 @@ git-config - Get and set repository or global options\n SYNOPSIS\n --------\n [verse]\n-'git config' [] [type] [-z|--null] name [value [value_regex]]\n+'git config' [] [type] [--show-origin] [-z|--null] name [value [value_regex]]\n 'git config' [] [type] --add name value\n 'git config' [] [type] --replace-all name value [value_regex]\n-'git config' [] [type] [-z|--null] --get name [value_regex]\n-'git config' [] [type] [-z|--null] --get-all name [value_regex]\n-'git config' [] [type] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]\n+'git config' [] [type] [--show-origin] [-z|--null] --get name [value_regex]\n+'git config' [] [type] [--show-origin] [-z|--null] --get-all name [value_regex]\n+'git config' [] [type] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]\n 'git config' [] [type] [-z|--null] --get-urlmatch name URL\n 'git config' [] --unset name [value_regex]\n 'git config' [] --unset-all name [value_regex]\n 'git config' [] --rename-section old_name new_name\n 'git config' [] --remove-section name\n-'git config' [] [-z|--null] [--name-only] -l | --list\n+'git config' [] [--show-origin] [-z|--null] [--name-only] -l | --list\n 'git config' [] --get-color name [default]\n 'git config' [] --get-colorbool name [stdout-is-tty]\n 'git config' [] -e | --edit\n@@ -194,6 +194,11 @@ See also <>.\n \tOutput only the names of config variables for `--list` or\n \t`--get-regexp`.\n \n+--show-origin::\n+\tAugment the output of all queried config options with the\n+\torigin type (file, stdin, blob, cmdline) and the actual origin\n+\t(config file path, ref, or blob id if applicable).\n+\n --get-colorbool name [stdout-is-tty]::\n \n \tFind the color setting for `name` (e.g. `color.diff`) and output\ndiff --git a/builtin/config.c b/builtin/config.c\nindex adc7727..7bad0c0 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -3,6 +3,7 @@\n #include \"color.h\"\n #include \"parse-options.h\"\n #include \"urlmatch.h\"\n+#include \"quote.h\"\n \n static const char *const builtin_config_usage[] = {\n \tN_(\"git config []\"),\n@@ -27,6 +28,7 @@ static int actions, types;\n static const char *get_color_slot, *get_colorbool_slot;\n static int end_null;\n static int respect_includes = -1;\n+static int show_origin;\n \n #define ACTION_GET (1<<0)\n #define ACTION_GET_ALL (1<<1)\n@@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {\n \tOPT_BOOL('z', \"null\", &end_null, N_(\"terminate values with NUL byte\")),\n \tOPT_BOOL(0, \"name-only\", &omit_values, N_(\"show variable names only\")),\n \tOPT_BOOL(0, \"includes\", &respect_includes, N_(\"respect include directives on lookup\")),\n+\tOPT_BOOL(0, \"show-origin\", &show_origin, N_(\"show origin of config (file, stdin, blob, cmdline)\")),\n \tOPT_END(),\n };\n \n@@ -91,8 +94,28 @@ static void check_argc(int argc, int min, int max) {\n \tusage_with_options(builtin_config_usage, builtin_config_options);\n }\n \n+static void show_config_origin(struct strbuf *buf)\n+{\n+\tconst char term = end_null ? '\\0' : '\\t';\n+\n+\tstrbuf_addstr(buf, current_config_type());\n+\tstrbuf_addch(buf, ':');\n+\tif (end_null)\n+\t\tstrbuf_addstr(buf, current_config_name());\n+\telse\n+\t\tquote_c_style(current_config_name(), buf, NULL, 0);\n+\tstrbuf_addch(buf, term);\n+}\n+\n static int show_all_config(const char *key_, const char *value_, void *cb)\n {\n+\tif (show_origin) {\n+\t\tstruct strbuf buf = STRBUF_INIT;\n+\t\tshow_config_origin(&buf);\n+\t\t/* Use fwrite as \"buf\" can contain \\0's if \"end_null\" is set. */\n+\t\tfwrite(buf.buf, 1, buf.len, stdout);\n+\t\tstrbuf_release(&buf);\n+\t}\n \tif (!omit_values && value_)\n \t\tprintf(\"%s%c%s%c\", key_, delim, value_, term);\n \telse\n@@ -108,6 +131,8 @@ struct strbuf_list {\n \n static int format_config(struct strbuf *buf, const char *key_, const char *value_)\n {\n+\tif (show_origin)\n+\t\tshow_config_origin(buf);\n \tif (show_keys)\n \t\tstrbuf_addstr(buf, key_);\n \tif (!omit_values) {\n@@ -538,6 +563,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\terror(\"--name-only is only applicable to --list or --get-regexp\");\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n+\n+\tif (show_origin && !(actions &\n+\t\t(ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST))) {\n+\t\terror(\"--show-origin is only applicable to --get, --get-all, \"\n+\t\t\t \"--get-regexp, and --list.\");\n+\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n+\t}\n+\n \tif (actions == ACTION_LIST) {\n \t\tcheck_argc(argc, 0, 0);\n \t\tif (git_config_with_options(show_all_config, NULL,\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 42ed5cc..4abbdf9 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1209,4 +1209,157 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '\n \t \"die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600\"\n '\n \n+test_expect_success 'set up --show-origin tests' '\n+\tINCLUDE_DIR=\"$HOME/include\" &&\n+\tmkdir -p \"$INCLUDE_DIR\" &&\n+\tcat >\"$INCLUDE_DIR\"/absolute.include <<-\\EOF &&\n+\t\t[user]\n+\t\t\tabsolute = include\n+\tEOF\n+\tcat >\"$INCLUDE_DIR\"/relative.include <<-\\EOF &&\n+\t\t[user]\n+\t\t\trelative = include\n+\tEOF\n+\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n+\t\t[user]\n+\t\t\tglobal = true\n+\t\t\toverride = global\n+\t\t[include]\n+\t\t\tpath = \"$INCLUDE_DIR/absolute.include\"\n+\tEOF\n+\tcat >.git/config <<-\\EOF\n+\t\t[user]\n+\t\t\tlocal = true\n+\t\t\toverride = local\n+\t\t[include]\n+\t\t\tpath = ../include/relative.include\n+\tEOF\n+'\n+\n+test_expect_success '--show-origin with --list' '\n+\tcat >expect <<-EOF &&\n+\t\tfile:$HOME/.gitconfig\tuser.global=true\n+\t\tfile:$HOME/.gitconfig\tuser.override=global\n+\t\tfile:$HOME/.gitconfig\tinclude.path=$INCLUDE_DIR/absolute.include\n+\t\tfile:$INCLUDE_DIR/absolute.include\tuser.absolute=include\n+\t\tfile:.git/config\tuser.local=true\n+\t\tfile:.git/config\tuser.override=local\n+\t\tfile:.git/config\tinclude.path=../include/relative.include\n+\t\tfile:.git/../include/relative.include\tuser.relative=include\n+\t\tcmdline:\tuser.cmdline=true\n+\tEOF\n+\tgit -c user.cmdline=true config --list --show-origin >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin with --list --null' '\n+\tcat >expect <<-EOF &&\n+\t\tfile:$HOME/.gitconfigQuser.global\n+\t\ttrueQfile:$HOME/.gitconfigQuser.override\n+\t\tglobalQfile:$HOME/.gitconfigQinclude.path\n+\t\t$INCLUDE_DIR/absolute.includeQfile:$INCLUDE_DIR/absolute.includeQuser.absolute\n+\t\tincludeQfile:.git/configQuser.local\n+\t\ttrueQfile:.git/configQuser.override\n+\t\tlocalQfile:.git/configQinclude.path\n+\t\t../include/relative.includeQfile:.git/../include/relative.includeQuser.relative\n+\t\tincludeQcmdline:Quser.cmdline\n+\t\ttrueQ\n+\tEOF\n+\tgit -c user.cmdline=true config --null --list --show-origin >output.raw &&\n+\tnul_to_q output &&\n+\t# The here-doc above adds a newline that the --null output would not\n+\t# include. Add it here to make the two comparable.\n+\techo >>output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin with single file' '\n+\tcat >expect <<-\\EOF &&\n+\t\tfile:.git/config\tuser.local=true\n+\t\tfile:.git/config\tuser.override=local\n+\t\tfile:.git/config\tinclude.path=../include/relative.include\n+\tEOF\n+\tgit config --local --list --show-origin >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin with --get-regexp' '\n+\tcat >expect <<-EOF &&\n+\t\tfile:$HOME/.gitconfig\tuser.global true\n+\t\tfile:.git/config\tuser.local true\n+\tEOF\n+\tgit config --show-origin --get-regexp \"user\\.[g|l].*\" >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin getting a single key' '\n+\tcat >expect <<-\\EOF &&\n+\t\tfile:.git/config\tlocal\n+\tEOF\n+\tgit config --show-origin user.override >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'set up custom config file' '\n+\tCUSTOM_CONFIG_FILE=$(printf \"file\\twith\\ttabs.conf\") &&\n+\tcat >\"$CUSTOM_CONFIG_FILE\" <<-\\EOF\n+\t\t[user]\n+\t\t\tcustom = true\n+\tEOF\n+'\n+\n+test_expect_success '--show-origin escape special file name characters' '\n+\tcat >expect <<-\\EOF &&\n+\t\tfile:\"file\\twith\\ttabs.conf\"\tuser.custom=true\n+\tEOF\n+\tgit config --file \"$CUSTOM_CONFIG_FILE\" --show-origin --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin stdin' '\n+\tcat >expect <<-\\EOF &&\n+\t\tstdin:\tuser.custom=true\n+\tEOF\n+\tgit config --file - --show-origin --list <\"$CUSTOM_CONFIG_FILE\" >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin stdin with file include' '\n+\tcat >\"$INCLUDE_DIR\"/stdin.include <<-EOF &&\n+\t\t[user]\n+\t\t\tstdin = include\n+\tEOF\n+\tcat >expect <<-EOF &&\n+\t\tfile:$INCLUDE_DIR/stdin.include\tinclude\n+\tEOF\n+\techo \"[include]path=\\\"$INCLUDE_DIR\\\"/stdin.include\" \\\n+\t\t| git config --show-origin --includes --file - user.stdin >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin blob' '\n+\tcat >expect <<-\\EOF &&\n+\t\tblob:a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08\tuser.custom=true\n+\tEOF\n+\tblob=$(git hash-object -w \"$CUSTOM_CONFIG_FILE\") &&\n+\tgit config --blob=$blob --show-origin --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success '--show-origin blob ref' '\n+\tcat >expect <<-\\EOF &&\n+\t\tblob:\"master:file\\twith\\ttabs.conf\"\tuser.custom=true\n+\tEOF\n+\tgit add \"$CUSTOM_CONFIG_FILE\" &&\n+\tgit commit -m \"new config file\" &&\n+\tgit config --blob=master:\"$CUSTOM_CONFIG_FILE\" --show-origin --list >output &&\n+\ttest_cmp expect output\n+'\n+\n+test_expect_success 'tear down --show-origin tests' '\n+\trm -rf \"$INCLUDE_DIR\" &&\n+\t>\"$HOME\"/.gitconfig &&\n+\t>.git/config\n+'\n+\n test_done\n-- \n2.5.1\n"},{"id":"278168","messageId":"20160215174157.GA26443@sigill.intra.peff.net","threadId":"41413","inReplyTo":"1455531466-16617-2-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 1/3] t: do not hide Git's exit code in tests","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T17:41:57Z","receivedAt":"2016-02-15T17:41:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 11:17:44AM +0100, larsxschneider@gmail.com wrote:\n\n> From: Lars Schneider \n> \n> Git should not be on the left-hand side of a pipe, because it hides the exit\n> code, and we want to make sure git does not fail.\n\nI think this is a nice cleanup.\n\n> There is one more occurrence of this pattern in t9010-svn-fe.sh which is to\n> evolved to change it easily.\n\nThe final sentence in the commit message needs s/to/too/. However, I'm\nnot sure this is the only remaining case. Doing:\n\n git grep -E 'git.*[^|]\\|($|[^|])'\n\nshows quite a few. I guess you just looked for \"nul_to_q\". There is\ncertainly no need to fix all of them, but you may want to note the\nextent of your grepping in your commit message.\n\n-Peff\n"},{"id":"278169","messageId":"20160215174218.GB26443@sigill.intra.peff.net","threadId":"41413","inReplyTo":"1455531466-16617-3-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T17:42:18Z","receivedAt":"2016-02-15T17:42:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 11:17:45AM +0100, larsxschneider@gmail.com wrote:\n\n> From: Lars Schneider \n> \n> Use the config type to print more detailed error messages that inform\n> the user about the origin of a config error (file, stdin, blob).\n> \n> Signed-off-by: Lars Schneider \n> ---\n> cache.h | 6 ++++--\n> config.c | 36 +++++++++++++++++++++++++-----------\n> submodule-config.c | 4 ++--\n> t/t1300-repo-config.sh | 8 +++++++-\n> t/t1308-config-set.sh | 4 ++--\n> 5 files changed, 40 insertions(+), 18 deletions(-)\n\nLooks good to me.\n\n-Peff\n"},{"id":"278176","messageId":"20160215180552.GC26443@sigill.intra.peff.net","threadId":"41413","inReplyTo":"1455531466-16617-1-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 0/3] config: add '--sources' option to print the source of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T18:05:52Z","receivedAt":"2016-02-15T18:05:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 11:17:43AM +0100, larsxschneider@gmail.com wrote:\n\n> I like the idea of a \"test set up block\" within a test script. In order\n> to clean up nicely before any subsequent tests I would like to propose\n> a \"tear down\" block. Would that work as a compromise in our \"test cases\n> depend on earlier test cases\" discussion?\n\nI don't have any real problem with what you've written in the final\npatch, but I also don't think it's accomplishing much (and is more lines\nof code, and more running processes).\n\nIf you want to run test N without having run all of 1..N-1, what you\nreally want is some known, reliable state when that test starts. But the\ntests before it do not necessarily know what that state is. The best\nthey can do is roughly restore the original state before they ran. But:\n\n 1. What does the state consist of? Which files (and their contents)\n are important to the test?\n\n In your tear-down you get rid of $INCLUDE_DIR, and you zero-out the\n config files. But you leave expect, output, output.raw, and the\n oddly named $CUSTOM_CONFIG_FILE. Nor do you clean up the\n environment variables.\n\n To be clear, I think it's perfectly fine to leave those. But you\n are still making assumptions about what the next test relies on.\n\n 2. We may create a clean slate, but that is probably not what the next\n test wants. It will want to do its own setup. I.e., it will\n probably not want a blank .git/config, and will create it itself,\n just as you did in your setup step.\n\nSo rather than tearing down, I think we are better off trying to make\ntests themselves (or blocks of them) set up their own assumptions. E.g.,\nby overwriting files rather than appending to them. By using unique\nfilenames, commit messages, etc for their tests. That's less of a big\ndeal here, but in many tests that create commits, \"test_commit foo\"\nwould fail a second time, because there are no changes to \"foo\". Doing\n\"test_commit subdir/check-diff-in-subdir\" is less likely to clash\nwithout another test.\n\nSometimes we _are_ better off with a teardown step, because subsequent\ntests would not reasonably think to clear some state we've set (e.g., in\nnon-config tests, if we set some random config variable, we use\ntest_config to tear it down afterwards rather than have each test clean\nout all of the config). So there's definitely a subjective judgement\ncall on what is \"reasonable\" there. But I find it unlikely that your\ntear-down will help anybody in this case. Further tests will not care\nabout $INCLUDE_DIR unless they reference it, and any further tests would\nset up their own .git/config, etc.\n\n-Peff\n"},{"id":"278177","messageId":"20160215180644.GD26443@sigill.intra.peff.net","threadId":"41413","inReplyTo":"1455531466-16617-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T18:06:44Z","receivedAt":"2016-02-15T18:06:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 11:17:46AM +0100, larsxschneider@gmail.com wrote:\n\n> From: Lars Schneider \n> \n> If config values are queried using 'git config' (e.g. via --get,\n> --get-all, --get-regexp, or --list flag) then it is sometimes hard to\n> find the configuration file where the values were defined.\n> \n> Teach 'git config' the '--show-origin' option to print the source\n> configuration file for every printed value.\n> \n> Based-on-patch-by: Jeff King \n> Signed-off-by: Lars Schneider \n> ---\n> Documentation/git-config.txt | 15 +++--\n> builtin/config.c | 33 ++++++++++\n> t/t1300-repo-config.sh | 153 +++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 196 insertions(+), 5 deletions(-)\n\nI packed all of my thoughts on test tear-down into my response to the\ncover letter. :)\n\nOther than that minor point, this version looks good to me.\n\n-Peff\n"},{"id":"278195","messageId":"CAPig+cS=TtVGTxo0oHny4GpAAaej93T2kP93upfHtPMkAd7V0g@mail.gmail.com","threadId":"41413","inReplyTo":"1455531466-16617-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2016-02-15T20:58:12Z","receivedAt":"2016-02-15T20:58:12Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Feb 15, 2016 at 5:17 AM, wrote:\n> If config values are queried using 'git config' (e.g. via --get,\n> --get-all, --get-regexp, or --list flag) then it is sometimes hard to\n> find the configuration file where the values were defined.\n>\n> Teach 'git config' the '--show-origin' option to print the source\n> configuration file for every printed value.\n>\n> Based-on-patch-by: Jeff King \n> Signed-off-by: Lars Schneider \n> ---\n> diff --git a/builtin/config.c b/builtin/config.c\n> @@ -27,6 +28,7 @@ static int actions, types;\n> static const char *get_color_slot, *get_colorbool_slot;\n> static int end_null;\n\nNot related to your changes, but I just realized that this variable\nreally ought to be named 'end_nul' since we're talking about the\ncharacter NUL, not a NULL pointer.\n\n> static int respect_includes = -1;\n> +static int show_origin;\n> @@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {\n> OPT_BOOL('z', \"null\", &end_null, N_(\"terminate values with NUL byte\")),\n\nLikewise, the long option name should be --nul rather than --null, or\nthe long name could be dropped altogether since some other commands\njust recognize short option -z.\n\nThere is no need for this patch series to address this anomaly; it's\nperhaps low-hanging fruit for someone wanting to join the project. The\nonly very minor wrinkle is that we'd still need to recognize --null as\na deprecated (and undocumented) alias for --nul.\n\n> OPT_BOOL(0, \"name-only\", &omit_values, N_(\"show variable names only\")),\n> OPT_BOOL(0, \"includes\", &respect_includes, N_(\"respect include directives on lookup\")),\n> + OPT_BOOL(0, \"show-origin\", &show_origin, N_(\"show origin of config (file, stdin, blob, cmdline)\")),\n> OPT_END(),\n> };\n"},{"id":"278196","messageId":"20160215210345.GA31008@sigill.intra.peff.net","threadId":"41413","inReplyTo":"CAPig+cS=TtVGTxo0oHny4GpAAaej93T2kP93upfHtPMkAd7V0g@mail.gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T21:03:46Z","receivedAt":"2016-02-15T21:03:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 03:58:12PM -0500, Eric Sunshine wrote:\n\n> > diff --git a/builtin/config.c b/builtin/config.c\n> > @@ -27,6 +28,7 @@ static int actions, types;\n> > static const char *get_color_slot, *get_colorbool_slot;\n> > static int end_null;\n> \n> Not related to your changes, but I just realized that this variable\n> really ought to be named 'end_nul' since we're talking about the\n> character NUL, not a NULL pointer.\n\nYeah, I noticed that, too. We just went through a round of related fixes\nhere, and the \"usual\" name is now \"nul_term_line\". I don't especially\nlike that one either, but at least it is correct and consistent. :)\n\n> > static int respect_includes = -1;\n> > +static int show_origin;\n> > @@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {\n> > OPT_BOOL('z', \"null\", &end_null, N_(\"terminate values with NUL byte\")),\n> \n> Likewise, the long option name should be --nul rather than --null, or\n> the long name could be dropped altogether since some other commands\n> just recognize short option -z.\n> \n> There is no need for this patch series to address this anomaly; it's\n> perhaps low-hanging fruit for someone wanting to join the project. The\n> only very minor wrinkle is that we'd still need to recognize --null as\n> a deprecated (and undocumented) alias for --nul.\n\nI think that would be OK as long as we keep the compatible option.\n\n-Peff\n"},{"id":"278199","messageId":"56C24375.4080007@ramsayjones.plus.com","threadId":"41413","inReplyTo":"1455531466-16617-3-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-15T21:30:29Z","receivedAt":"2016-02-15T21:30:29Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 15/02/16 10:17, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n> \n> Use the config type to print more detailed error messages that inform\n> the user about the origin of a config error (file, stdin, blob).\n> \n> Signed-off-by: Lars Schneider \n> ---\n> cache.h | 6 ++++--\n> config.c | 36 +++++++++++++++++++++++++-----------\n> submodule-config.c | 4 ++--\n> t/t1300-repo-config.sh | 8 +++++++-\n> t/t1308-config-set.sh | 4 ++--\n> 5 files changed, 40 insertions(+), 18 deletions(-)\n> \n> diff --git a/cache.h b/cache.h\n> index c63fcc1..8d86e5c 100644\n> --- a/cache.h\n> +++ b/cache.h\n> @@ -1485,8 +1485,8 @@ struct git_config_source {\n> typedef int (*config_fn_t)(const char *, const char *, void *);\n> extern int git_default_config(const char *, const char *, void *);\n> extern int git_config_from_file(config_fn_t fn, const char *, void *);\n> -extern int git_config_from_buf(config_fn_t fn, const char *name,\n> -\t\t\t const char *buf, size_t len, void *data);\n> +extern int git_config_from_mem(config_fn_t fn, const char *type,\n> +\t\t\t\t\tconst char *name, const char *buf, size_t len, void *data);\n> extern void git_config_push_parameter(const char *text);\n> extern int git_config_from_parameters(config_fn_t fn, void *data);\n> extern void git_config(config_fn_t fn, void *);\n> @@ -1525,6 +1525,8 @@ extern const char *get_log_output_encoding(void);\n> extern const char *get_commit_output_encoding(void);\n> \n> extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n> +extern const char *current_config_type();\n> +extern const char *current_config_name();\n> \n> struct config_include_data {\n> \tint depth;\n> diff --git a/config.c b/config.c\n> index 86a5eb2..5cf11b5 100644\n> --- a/config.c\n> +++ b/config.c\n> @@ -24,6 +24,7 @@ struct config_source {\n> \t\t\tsize_t pos;\n> \t\t} buf;\n> \t} u;\n> +\tconst char *type;\n> \tconst char *name;\n> \tconst char *path;\n> \tint die_on_error;\n> @@ -471,9 +472,9 @@ static int git_parse_source(config_fn_t fn, void *data)\n> \t\t\tbreak;\n> \t}\n> \tif (cf->die_on_error)\n> -\t\tdie(_(\"bad config file line %d in %s\"), cf->linenr, cf->name);\n> +\t\tdie(_(\"bad config line %d in %s %s\"), cf->linenr, cf->type, cf->name);\n> \telse\n> -\t\treturn error(_(\"bad config file line %d in %s\"), cf->linenr, cf->name);\n> +\t\treturn error(_(\"bad config line %d in %s %s\"), cf->linenr, cf->type, cf->name);\n> }\n> \n> static int parse_unit_factor(const char *end, uintmax_t *val)\n> @@ -588,9 +589,9 @@ static void die_bad_number(const char *name, const char *value)\n> \tif (!value)\n> \t\tvalue = \"\";\n> \n> -\tif (cf && cf->name)\n> -\t\tdie(_(\"bad numeric config value '%s' for '%s' in %s: %s\"),\n> -\t\t value, name, cf->name, reason);\n> +\tif (cf && cf->type && cf->name)\n> +\t\tdie(_(\"bad numeric config value '%s' for '%s' in %s %s: %s\"),\n> +\t\t value, name, cf->type, cf->name, reason);\n> \tdie(_(\"bad numeric config value '%s' for '%s': %s\"), value, name, reason);\n> }\n> \n> @@ -1061,11 +1062,13 @@ static int do_config_from(struct config_source *top, config_fn_t fn, void *data)\n> }\n> \n> static int do_config_from_file(config_fn_t fn,\n> -\t\tconst char *name, const char *path, FILE *f, void *data)\n> +\t\tconst char *type, const char *name, const char *path, FILE *f,\n> +\t\tvoid *data)\n> {\n> \tstruct config_source top;\n> \n> \ttop.u.file = f;\n> +\ttop.type = type;\n> \ttop.name = name;\n> \ttop.path = path;\n> \ttop.die_on_error = 1;\n> @@ -1078,7 +1081,7 @@ static int do_config_from_file(config_fn_t fn,\n> \n> static int git_config_from_stdin(config_fn_t fn, void *data)\n> {\n> -\treturn do_config_from_file(fn, \"\", NULL, stdin, data);\n> +\treturn do_config_from_file(fn, \"stdin\", \"\", NULL, stdin, data);\n\nI think this should be:\n\treturn do_config_from_file(fn, \"file\", \"\", NULL, stdin, data);\n\nie. is not a separate type, but a file that does not exist in\nthe filesystem and, thus, has no name. (what you use internally is a\nseparate issue, but works for me.)\n\nAlso, I'm so used to '' as the 'name' (token/handle) of that\nfile, that it looks very odd spelt otherwise. :-D\n\n> }\n> \n> int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n> @@ -1089,21 +1092,22 @@ int git_config_from_file(config_fn_t fn, const char *filename, void *data)\n> \tf = fopen(filename, \"r\");\n> \tif (f) {\n> \t\tflockfile(f);\n> -\t\tret = do_config_from_file(fn, filename, filename, f, data);\n> +\t\tret = do_config_from_file(fn, \"file\", filename, filename, f, data);\n> \t\tfunlockfile(f);\n> \t\tfclose(f);\n> \t}\n> \treturn ret;\n> }\n> \n> -int git_config_from_buf(config_fn_t fn, const char *name, const char *buf,\n> -\t\t\tsize_t len, void *data)\n> +int git_config_from_mem(config_fn_t fn, const char *type, const char *name,\n> +\t\t\tconst char *buf, size_t len, void *data)\n> {\n> \tstruct config_source top;\n> \n> \ttop.u.buf.buf = buf;\n> \ttop.u.buf.len = len;\n> \ttop.u.buf.pos = 0;\n> +\ttop.type = type;\n> \ttop.name = name;\n> \ttop.path = NULL;\n> \ttop.die_on_error = 0;\n> @@ -1132,7 +1136,7 @@ static int git_config_from_blob_sha1(config_fn_t fn,\n> \t\treturn error(\"reference '%s' does not point to a blob\", name);\n> \t}\n> \n> -\tret = git_config_from_buf(fn, name, buf, size, data);\n> +\tret = git_config_from_mem(fn, \"blob\", name, buf, size, data);\n> \tfree(buf);\n> \n> \treturn ret;\n> @@ -2385,3 +2389,13 @@ int parse_config_key(const char *var,\n> \n> \treturn 0;\n> }\n> +\n> +const char *current_config_type()\n> +{\n> +\treturn cf && cf->type ? cf->type : \"cmdline\";\n> +}\n> +\n> +const char *current_config_name()\n> +{\n> +\treturn cf && cf->name ? cf->name : \"\";\n> +}\n> diff --git a/submodule-config.c b/submodule-config.c\n> index fe8ceab..92502b5 100644\n> --- a/submodule-config.c\n> +++ b/submodule-config.c\n> @@ -427,8 +427,8 @@ static const struct submodule *config_from(struct submodule_cache *cache,\n> \tparameter.commit_sha1 = commit_sha1;\n> \tparameter.gitmodules_sha1 = sha1;\n> \tparameter.overwrite = 0;\n> -\tgit_config_from_buf(parse_config, rev.buf, config, config_size,\n> -\t\t\t¶meter);\n> +\tgit_config_from_mem(parse_config, \"submodule-blob\", rev.buf,\n> +\t\t\tconfig, config_size, ¶meter);\n> \tfree(config);\n> \n> \tswitch (lookup_type) {\n> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n> index 1782add..42ed5cc 100755\n> --- a/t/t1300-repo-config.sh\n> +++ b/t/t1300-repo-config.sh\n> @@ -700,12 +700,18 @@ test_expect_success 'invalid unit' '\n> \tgit config aninvalid.unit >actual &&\n> \ttest_cmp expect actual &&\n> \tcat >expect <<-\\EOF &&\n> -\tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'' in .git/config: invalid unit\n> +\tfatal: bad numeric config value '\\''1auto'\\'' for '\\''aninvalid.unit'\\'' in file .git/config: invalid unit\n> \tEOF\n> \ttest_must_fail git config --int --get aninvalid.unit 2>actual &&\n> \ttest_i18ncmp expect actual\n> '\n> \n> +test_expect_success 'invalid stdin config' '\n> +\techo \"fatal: bad config line 1 in stdin \" >expect &&\n\nSo, this looks odd. Using the above would give:\n\tfatal: bad config line 1 in file \n\n> +\techo \"[broken\" | test_must_fail git config --list --file - >output 2>&1 &&\n> +\ttest_cmp expect output\n> +'\n> +\n> cat > expect << EOF\n> true\n> false\n> diff --git a/t/t1308-config-set.sh b/t/t1308-config-set.sh\n> index 91235b7..82f82a1 100755\n> --- a/t/t1308-config-set.sh\n> +++ b/t/t1308-config-set.sh\n> @@ -195,14 +195,14 @@ test_expect_success 'proper error on error in default config files' '\n> \tcp .git/config .git/config.old &&\n> \ttest_when_finished \"mv .git/config.old .git/config\" &&\n> \techo \"[\" >>.git/config &&\n> -\techo \"fatal: bad config file line 34 in .git/config\" >expect &&\n> +\techo \"fatal: bad config line 34 in file .git/config\" >expect &&\n> \ttest_expect_code 128 test-config get_value foo.bar 2>actual &&\n> \ttest_cmp expect actual\n> '\n> \n> test_expect_success 'proper error on error in custom config files' '\n> \techo \"[\" >>syntax-error &&\n> -\techo \"fatal: bad config file line 1 in syntax-error\" >expect &&\n> +\techo \"fatal: bad config line 1 in file syntax-error\" >expect &&\n> \ttest_expect_code 128 test-config configset_get_value foo.bar syntax-error 2>actual &&\n> \ttest_cmp expect actual\n> '\n> \n\nATB,\nRamsay Jones\n"},{"id":"278202","messageId":"56C244D7.1030503@ramsayjones.plus.com","threadId":"41413","inReplyTo":"1455531466-16617-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-15T21:36:23Z","receivedAt":"2016-02-15T21:36:23Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 15/02/16 10:17, larsxschneider@gmail.com wrote:\n> From: Lars Schneider \n> \n> If config values are queried using 'git config' (e.g. via --get,\n> --get-all, --get-regexp, or --list flag) then it is sometimes hard to\n> find the configuration file where the values were defined.\n> \n> Teach 'git config' the '--show-origin' option to print the source\n> configuration file for every printed value.\n> \n> Based-on-patch-by: Jeff King \n> Signed-off-by: Lars Schneider \n> ---\n> Documentation/git-config.txt | 15 +++--\n> builtin/config.c | 33 ++++++++++\n> t/t1300-repo-config.sh | 153 +++++++++++++++++++++++++++++++++++++++++++\n> 3 files changed, 196 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> index 2608ca7..6374997 100644\n> --- a/Documentation/git-config.txt\n> +++ b/Documentation/git-config.txt\n> @@ -9,18 +9,18 @@ git-config - Get and set repository or global options\n> SYNOPSIS\n> --------\n> [verse]\n> -'git config' [] [type] [-z|--null] name [value [value_regex]]\n> +'git config' [] [type] [--show-origin] [-z|--null] name [value [value_regex]]\n> 'git config' [] [type] --add name value\n> 'git config' [] [type] --replace-all name value [value_regex]\n> -'git config' [] [type] [-z|--null] --get name [value_regex]\n> -'git config' [] [type] [-z|--null] --get-all name [value_regex]\n> -'git config' [] [type] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]\n> +'git config' [] [type] [--show-origin] [-z|--null] --get name [value_regex]\n> +'git config' [] [type] [--show-origin] [-z|--null] --get-all name [value_regex]\n> +'git config' [] [type] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]\n> 'git config' [] [type] [-z|--null] --get-urlmatch name URL\n> 'git config' [] --unset name [value_regex]\n> 'git config' [] --unset-all name [value_regex]\n> 'git config' [] --rename-section old_name new_name\n> 'git config' [] --remove-section name\n> -'git config' [] [-z|--null] [--name-only] -l | --list\n> +'git config' [] [--show-origin] [-z|--null] [--name-only] -l | --list\n> 'git config' [] --get-color name [default]\n> 'git config' [] --get-colorbool name [stdout-is-tty]\n> 'git config' [] -e | --edit\n> @@ -194,6 +194,11 @@ See also <>.\n> \tOutput only the names of config variables for `--list` or\n> \t`--get-regexp`.\n> \n> +--show-origin::\n> +\tAugment the output of all queried config options with the\n> +\torigin type (file, stdin, blob, cmdline) and the actual origin\n\nfile, blob, cmdline (hmm, maybe cmd-line? ;-) )\n\n> +\t(config file path, ref, or blob id if applicable).\n> +\n> --get-colorbool name [stdout-is-tty]::\n> \n> \tFind the color setting for `name` (e.g. `color.diff`) and output\n> diff --git a/builtin/config.c b/builtin/config.c\n> index adc7727..7bad0c0 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -3,6 +3,7 @@\n> #include \"color.h\"\n> #include \"parse-options.h\"\n> #include \"urlmatch.h\"\n> +#include \"quote.h\"\n> \n> static const char *const builtin_config_usage[] = {\n> \tN_(\"git config []\"),\n> @@ -27,6 +28,7 @@ static int actions, types;\n> static const char *get_color_slot, *get_colorbool_slot;\n> static int end_null;\n> static int respect_includes = -1;\n> +static int show_origin;\n> \n> #define ACTION_GET (1<<0)\n> #define ACTION_GET_ALL (1<<1)\n> @@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {\n> \tOPT_BOOL('z', \"null\", &end_null, N_(\"terminate values with NUL byte\")),\n> \tOPT_BOOL(0, \"name-only\", &omit_values, N_(\"show variable names only\")),\n> \tOPT_BOOL(0, \"includes\", &respect_includes, N_(\"respect include directives on lookup\")),\n> +\tOPT_BOOL(0, \"show-origin\", &show_origin, N_(\"show origin of config (file, stdin, blob, cmdline)\")),\n> \tOPT_END(),\n> };\n> \n> @@ -91,8 +94,28 @@ static void check_argc(int argc, int min, int max) {\n> \tusage_with_options(builtin_config_usage, builtin_config_options);\n> }\n> \n> +static void show_config_origin(struct strbuf *buf)\n> +{\n> +\tconst char term = end_null ? '\\0' : '\\t';\n> +\n> +\tstrbuf_addstr(buf, current_config_type());\n> +\tstrbuf_addch(buf, ':');\n> +\tif (end_null)\n> +\t\tstrbuf_addstr(buf, current_config_name());\n> +\telse\n> +\t\tquote_c_style(current_config_name(), buf, NULL, 0);\n> +\tstrbuf_addch(buf, term);\n> +}\n> +\n> static int show_all_config(const char *key_, const char *value_, void *cb)\n> {\n> +\tif (show_origin) {\n> +\t\tstruct strbuf buf = STRBUF_INIT;\n> +\t\tshow_config_origin(&buf);\n> +\t\t/* Use fwrite as \"buf\" can contain \\0's if \"end_null\" is set. */\n> +\t\tfwrite(buf.buf, 1, buf.len, stdout);\n> +\t\tstrbuf_release(&buf);\n> +\t}\n> \tif (!omit_values && value_)\n> \t\tprintf(\"%s%c%s%c\", key_, delim, value_, term);\n> \telse\n> @@ -108,6 +131,8 @@ struct strbuf_list {\n> \n> static int format_config(struct strbuf *buf, const char *key_, const char *value_)\n> {\n> +\tif (show_origin)\n> +\t\tshow_config_origin(buf);\n> \tif (show_keys)\n> \t\tstrbuf_addstr(buf, key_);\n> \tif (!omit_values) {\n> @@ -538,6 +563,14 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n> \t\terror(\"--name-only is only applicable to --list or --get-regexp\");\n> \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n> \t}\n> +\n> +\tif (show_origin && !(actions &\n> +\t\t(ACTION_GET|ACTION_GET_ALL|ACTION_GET_REGEXP|ACTION_LIST))) {\n> +\t\terror(\"--show-origin is only applicable to --get, --get-all, \"\n> +\t\t\t \"--get-regexp, and --list.\");\n> +\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n> +\t}\n> +\n> \tif (actions == ACTION_LIST) {\n> \t\tcheck_argc(argc, 0, 0);\n> \t\tif (git_config_with_options(show_all_config, NULL,\n> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n> index 42ed5cc..4abbdf9 100755\n> --- a/t/t1300-repo-config.sh\n> +++ b/t/t1300-repo-config.sh\n> @@ -1209,4 +1209,157 @@ test_expect_success POSIXPERM,PERL 'preserves existing permissions' '\n> \t \"die q(badrename) if ((stat(q(.git/config)))[2] & 07777) != 0600\"\n> '\n> \n> +test_expect_success 'set up --show-origin tests' '\n> +\tINCLUDE_DIR=\"$HOME/include\" &&\n> +\tmkdir -p \"$INCLUDE_DIR\" &&\n> +\tcat >\"$INCLUDE_DIR\"/absolute.include <<-\\EOF &&\n> +\t\t[user]\n> +\t\t\tabsolute = include\n> +\tEOF\n> +\tcat >\"$INCLUDE_DIR\"/relative.include <<-\\EOF &&\n> +\t\t[user]\n> +\t\t\trelative = include\n> +\tEOF\n> +\tcat >\"$HOME\"/.gitconfig <<-EOF &&\n> +\t\t[user]\n> +\t\t\tglobal = true\n> +\t\t\toverride = global\n> +\t\t[include]\n> +\t\t\tpath = \"$INCLUDE_DIR/absolute.include\"\n> +\tEOF\n> +\tcat >.git/config <<-\\EOF\n> +\t\t[user]\n> +\t\t\tlocal = true\n> +\t\t\toverride = local\n> +\t\t[include]\n> +\t\t\tpath = ../include/relative.include\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--show-origin with --list' '\n> +\tcat >expect <<-EOF &&\n> +\t\tfile:$HOME/.gitconfig\tuser.global=true\n> +\t\tfile:$HOME/.gitconfig\tuser.override=global\n> +\t\tfile:$HOME/.gitconfig\tinclude.path=$INCLUDE_DIR/absolute.include\n> +\t\tfile:$INCLUDE_DIR/absolute.include\tuser.absolute=include\n> +\t\tfile:.git/config\tuser.local=true\n> +\t\tfile:.git/config\tuser.override=local\n> +\t\tfile:.git/config\tinclude.path=../include/relative.include\n> +\t\tfile:.git/../include/relative.include\tuser.relative=include\n> +\t\tcmdline:\tuser.cmdline=true\n> +\tEOF\n> +\tgit -c user.cmdline=true config --list --show-origin >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin with --list --null' '\n> +\tcat >expect <<-EOF &&\n> +\t\tfile:$HOME/.gitconfigQuser.global\n> +\t\ttrueQfile:$HOME/.gitconfigQuser.override\n> +\t\tglobalQfile:$HOME/.gitconfigQinclude.path\n> +\t\t$INCLUDE_DIR/absolute.includeQfile:$INCLUDE_DIR/absolute.includeQuser.absolute\n> +\t\tincludeQfile:.git/configQuser.local\n> +\t\ttrueQfile:.git/configQuser.override\n> +\t\tlocalQfile:.git/configQinclude.path\n> +\t\t../include/relative.includeQfile:.git/../include/relative.includeQuser.relative\n> +\t\tincludeQcmdline:Quser.cmdline\n> +\t\ttrueQ\n> +\tEOF\n> +\tgit -c user.cmdline=true config --null --list --show-origin >output.raw &&\n> +\tnul_to_q output &&\n> +\t# The here-doc above adds a newline that the --null output would not\n> +\t# include. Add it here to make the two comparable.\n> +\techo >>output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin with single file' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tfile:.git/config\tuser.local=true\n> +\t\tfile:.git/config\tuser.override=local\n> +\t\tfile:.git/config\tinclude.path=../include/relative.include\n> +\tEOF\n> +\tgit config --local --list --show-origin >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin with --get-regexp' '\n> +\tcat >expect <<-EOF &&\n> +\t\tfile:$HOME/.gitconfig\tuser.global true\n> +\t\tfile:.git/config\tuser.local true\n> +\tEOF\n> +\tgit config --show-origin --get-regexp \"user\\.[g|l].*\" >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin getting a single key' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tfile:.git/config\tlocal\n> +\tEOF\n> +\tgit config --show-origin user.override >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success 'set up custom config file' '\n> +\tCUSTOM_CONFIG_FILE=$(printf \"file\\twith\\ttabs.conf\") &&\n> +\tcat >\"$CUSTOM_CONFIG_FILE\" <<-\\EOF\n> +\t\t[user]\n> +\t\t\tcustom = true\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--show-origin escape special file name characters' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tfile:\"file\\twith\\ttabs.conf\"\tuser.custom=true\n> +\tEOF\n> +\tgit config --file \"$CUSTOM_CONFIG_FILE\" --show-origin --list >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin stdin' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tstdin:\tuser.custom=true\n\nSo, as with the previous patch, I think this should be:\n\t\tfile:\tuser.custom=true\n\nATB,\nRamsay Jones\n\n\n> +\tEOF\n> +\tgit config --file - --show-origin --list <\"$CUSTOM_CONFIG_FILE\" >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin stdin with file include' '\n> +\tcat >\"$INCLUDE_DIR\"/stdin.include <<-EOF &&\n> +\t\t[user]\n> +\t\t\tstdin = include\n> +\tEOF\n> +\tcat >expect <<-EOF &&\n> +\t\tfile:$INCLUDE_DIR/stdin.include\tinclude\n> +\tEOF\n> +\techo \"[include]path=\\\"$INCLUDE_DIR\\\"/stdin.include\" \\\n> +\t\t| git config --show-origin --includes --file - user.stdin >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin blob' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tblob:a9d9f9e555b5c6f07cbe09d3f06fe3df11e09c08\tuser.custom=true\n> +\tEOF\n> +\tblob=$(git hash-object -w \"$CUSTOM_CONFIG_FILE\") &&\n> +\tgit config --blob=$blob --show-origin --list >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success '--show-origin blob ref' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tblob:\"master:file\\twith\\ttabs.conf\"\tuser.custom=true\n> +\tEOF\n> +\tgit add \"$CUSTOM_CONFIG_FILE\" &&\n> +\tgit commit -m \"new config file\" &&\n> +\tgit config --blob=master:\"$CUSTOM_CONFIG_FILE\" --show-origin --list >output &&\n> +\ttest_cmp expect output\n> +'\n> +\n> +test_expect_success 'tear down --show-origin tests' '\n> +\trm -rf \"$INCLUDE_DIR\" &&\n> +\t>\"$HOME\"/.gitconfig &&\n> +\t>.git/config\n> +'\n> +\n> test_done\n> \n"},{"id":"278205","messageId":"20160215214049.GA10094@sigill.intra.peff.net","threadId":"41413","inReplyTo":"56C244D7.1030503@ramsayjones.plus.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T21:40:49Z","receivedAt":"2016-02-15T21:40:49Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 09:36:23PM +0000, Ramsay Jones wrote:\n\n> > +test_expect_success '--show-origin stdin' '\n> > +\tcat >expect <<-\\EOF &&\n> > +\t\tstdin:\tuser.custom=true\n> \n> So, as with the previous patch, I think this should be:\n> \t\tfile:\tuser.custom=true\n\nThat's ambiguous with a file named \"\", which was the point of\nhaving the two separate prefixes in the first place.\n\nI think in practice we _could_ get by with an ambiguous output (it's not\nlike \"\" is a common filename), but that was discussed earlier in\nthe thread, and Lars decided to go for something unambiguous.\n\nThat doesn't necessarily have to bleed over into the error messages,\nthough (which could continue to use \"\" if we want to put in a\nlittle extra code to covering the cases separately.\n\n-Peff\n"},{"id":"278207","messageId":"xmqqio1pps64.fsf@gitster.mtv.corp.google.com","threadId":"41413","inReplyTo":"56C244D7.1030503@ramsayjones.plus.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-15T21:41:55Z","receivedAt":"2016-02-15T21:41:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n>> +--show-origin::\n>> +\tAugment the output of all queried config options with the\n>> +\torigin type (file, stdin, blob, cmdline) and the actual origin\n>\n> file, blob, cmdline (hmm, maybe cmd-line? ;-) )\n\nIf we are going to spell it out, \"command-line\" would be the way to\ngo. \"cmdline\" is probably OK.\n"},{"id":"278233","messageId":"xmqqegcdpqhh.fsf@gitster.mtv.corp.google.com","threadId":"41413","inReplyTo":"1455531466-16617-4-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-15T22:18:18Z","receivedAt":"2016-02-15T22:18:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"larsxschneider@gmail.com writes:\n\n> +test_expect_success 'set up custom config file' '\n> +\tCUSTOM_CONFIG_FILE=$(printf \"file\\twith\\ttabs.conf\") &&\n> +\tcat >\"$CUSTOM_CONFIG_FILE\" <<-\\EOF\n> +\t\t[user]\n> +\t\t\tcustom = true\n> +\tEOF\n> +'\n> +\n> +test_expect_success '--show-origin escape special file name characters' '\n> +\tcat >expect <<-\\EOF &&\n> +\t\tfile:\"file\\twith\\ttabs.conf\"\tuser.custom=true\n> +\tEOF\n> +\tgit config --file \"$CUSTOM_CONFIG_FILE\" --show-origin --list >output &&\n> +\ttest_cmp expect output\n> +'\n\nDo we really need to use a file with such a name?\n\nAn existing test t3300 tells me that a test that uses a path with a\ntab needs to be skipped on FAT/NTFS. If your goal is to make sure\ndquote is exercised, can't we just do with a path with a SP in it or\nsomething?\n\nThanks.\n"},{"id":"278237","messageId":"56C253B8.1070702@ramsayjones.plus.com","threadId":"41413","inReplyTo":"20160215214049.GA10094@sigill.intra.peff.net","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-15T22:39:52Z","receivedAt":"2016-02-15T22:39:52Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 15/02/16 21:40, Jeff King wrote:\n> On Mon, Feb 15, 2016 at 09:36:23PM +0000, Ramsay Jones wrote:\n> \n>>> +test_expect_success '--show-origin stdin' '\n>>> +\tcat >expect <<-\\EOF &&\n>>> +\t\tstdin:\tuser.custom=true\n>>\n>> So, as with the previous patch, I think this should be:\n>> \t\tfile:\tuser.custom=true\n> \n> That's ambiguous with a file named \"\", which was the point of\n> having the two separate prefixes in the first place.\n> \n> I think in practice we _could_ get by with an ambiguous output (it's not\n> like \"\" is a common filename), but that was discussed earlier in\n> the thread, and Lars decided to go for something unambiguous.\n\nsure, I just don't think it would cause a problem in practice.\nHow about using '-' for ? Hmm, you can actually create\nsuch a file in the filesystem! Oh well, I guess its not a big deal.\n\n> \n> That doesn't necessarily have to bleed over into the error messages,\n> though (which could continue to use \"\" if we want to put in a\n> little extra code to covering the cases separately.\n\nYep.\n\nATB,\nRamsay Jones\n"},{"id":"278241","messageId":"20160215225938.GA30631@sigill.intra.peff.net","threadId":"41413","inReplyTo":"xmqqegcdpqhh.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-15T22:59:38Z","receivedAt":"2016-02-15T22:59:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Feb 15, 2016 at 02:18:18PM -0800, Junio C Hamano wrote:\n\n> larsxschneider@gmail.com writes:\n> \n> > +test_expect_success 'set up custom config file' '\n> > +\tCUSTOM_CONFIG_FILE=$(printf \"file\\twith\\ttabs.conf\") &&\n> > +\tcat >\"$CUSTOM_CONFIG_FILE\" <<-\\EOF\n> > +\t\t[user]\n> > +\t\t\tcustom = true\n> > +\tEOF\n> > +'\n> > +\n> > +test_expect_success '--show-origin escape special file name characters' '\n> > +\tcat >expect <<-\\EOF &&\n> > +\t\tfile:\"file\\twith\\ttabs.conf\"\tuser.custom=true\n> > +\tEOF\n> > +\tgit config --file \"$CUSTOM_CONFIG_FILE\" --show-origin --list >output &&\n> > +\ttest_cmp expect output\n> > +'\n> \n> Do we really need to use a file with such a name?\n> \n> An existing test t3300 tells me that a test that uses a path with a\n> tab needs to be skipped on FAT/NTFS. If your goal is to make sure\n> dquote is exercised, can't we just do with a path with a SP in it or\n> something?\n\nIt has to trigger quote_c_style(). You can see the complete set of\nquoted characters in quote.c:sq_lookup, but space is not one of them.\nProbably double-quote or backslash is the best bet, as the rest are all\ncontrol characters.\n\n-Peff\n"},{"id":"278249","messageId":"xmqq4md9o7da.fsf@gitster.mtv.corp.google.com","threadId":"41413","inReplyTo":"20160215225938.GA30631@sigill.intra.peff.net","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-15T23:56:33Z","receivedAt":"2016-02-15T23:56:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> An existing test t3300 tells me that a test that uses a path with a\n>> tab needs to be skipped on FAT/NTFS. If your goal is to make sure\n>> dquote is exercised, can't we just do with a path with a SP in it or\n>> something?\n>\n> It has to trigger quote_c_style(). You can see the complete set of\n> quoted characters in quote.c:sq_lookup, but space is not one of them.\n> Probably double-quote or backslash is the best bet, as the rest are all\n> control characters.\n\nYeah, 3300 seems to use dq for that.\n\nThanks for checking.\n"},{"id":"278285","messageId":"C99C00B1-B782-4CCD-A1B5-B83CE50DE6C8@gmail.com","threadId":"41413","inReplyTo":"20160215174157.GA26443@sigill.intra.peff.net","subject":"Re: [PATCH v4 1/3] t: do not hide Git's exit code in tests","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-16T09:36:42Z","receivedAt":"2016-02-16T09:36:42Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Feb 2016, at 18:41, Jeff King wrote:\n\n> On Mon, Feb 15, 2016 at 11:17:44AM +0100, larsxschneider@gmail.com wrote:\n> \n>> From: Lars Schneider \n>> \n>> Git should not be on the left-hand side of a pipe, because it hides the exit\n>> code, and we want to make sure git does not fail.\n> \n> I think this is a nice cleanup.\n> \n>> There is one more occurrence of this pattern in t9010-svn-fe.sh which is to\n>> evolved to change it easily.\n> \n> The final sentence in the commit message needs s/to/too/.\nOK!\n\n> However, I'm\n> not sure this is the only remaining case. Doing:\n> \n> git grep -E 'git.*[^|]\\|($|[^|])'\n> \n> shows quite a few. I guess you just looked for \"nul_to_q\". There is\n> certainly no need to fix all of them, but you may want to note the\n> extent of your grepping in your commit message.\nYes, I looked only for nul_to_q... there are more than 1000 other cases. \nI thought about a clever sed script to fix the easy ones but I don't\nthink it is worth the effort.\n\nThanks,\nLars\n"},{"id":"278286","messageId":"186152EC-C19A-464E-A81B-B614E56AF1DE@gmail.com","threadId":"41413","inReplyTo":"20160215180552.GC26443@sigill.intra.peff.net","subject":"Re: [PATCH v4 0/3] config: add '--sources' option to print the source of a config value","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-16T09:40:34Z","receivedAt":"2016-02-16T09:40:34Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Feb 2016, at 19:05, Jeff King wrote:\n\n> On Mon, Feb 15, 2016 at 11:17:43AM +0100, larsxschneider@gmail.com wrote:\n> \n>> I like the idea of a \"test set up block\" within a test script. In order\n>> to clean up nicely before any subsequent tests I would like to propose\n>> a \"tear down\" block. Would that work as a compromise in our \"test cases\n>> depend on earlier test cases\" discussion?\n> \n> I don't have any real problem with what you've written in the final\n> patch, but I also don't think it's accomplishing much (and is more lines\n> of code, and more running processes).\n> \n> If you want to run test N without having run all of 1..N-1, what you\n> really want is some known, reliable state when that test starts. But the\n> tests before it do not necessarily know what that state is. The best\n> they can do is roughly restore the original state before they ran. But:\n> \n> 1. What does the state consist of? Which files (and their contents)\n> are important to the test?\n> \n> In your tear-down you get rid of $INCLUDE_DIR, and you zero-out the\n> config files. But you leave expect, output, output.raw, and the\n> oddly named $CUSTOM_CONFIG_FILE. Nor do you clean up the\n> environment variables.\nGood argument - I can't disagree.\n\n> \n> To be clear, I think it's perfectly fine to leave those. But you\n> are still making assumptions about what the next test relies on.\n> \n> 2. We may create a clean slate, but that is probably not what the next\n> test wants. It will want to do its own setup. I.e., it will\n> probably not want a blank .git/config, and will create it itself,\n> just as you did in your setup step.\n> \n> So rather than tearing down, I think we are better off trying to make\n> tests themselves (or blocks of them) set up their own assumptions. E.g.,\n> by overwriting files rather than appending to them. By using unique\n> filenames, commit messages, etc for their tests. That's less of a big\n> deal here, but in many tests that create commits, \"test_commit foo\"\n> would fail a second time, because there are no changes to \"foo\". Doing\n> \"test_commit subdir/check-diff-in-subdir\" is less likely to clash\n> without another test.\n> \n> Sometimes we _are_ better off with a teardown step, because subsequent\n> tests would not reasonably think to clear some state we've set (e.g., in\n> non-config tests, if we set some random config variable, we use\n> test_config to tear it down afterwards rather than have each test clean\n> out all of the config). So there's definitely a subjective judgement\n> call on what is \"reasonable\" there. But I find it unlikely that your\n> tear-down will help anybody in this case. Further tests will not care\n> about $INCLUDE_DIR unless they reference it, and any further tests would\n> set up their own .git/config, etc.\n\nOK, I will remove the block in the next roll. Thanks for explaining\nyour thoughts on this.\n\n- Lars\n"},{"id":"278292","messageId":"BB209409-A43C-4C08-8691-B27FFDBDDB69@gmail.com","threadId":"41413","inReplyTo":"xmqqio1pps64.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-16T09:48:30Z","receivedAt":"2016-02-16T09:48:30Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Feb 2016, at 22:41, Junio C Hamano wrote:\n\n> Ramsay Jones writes:\n> \n>>> +--show-origin::\n>>> +\tAugment the output of all queried config options with the\n>>> +\torigin type (file, stdin, blob, cmdline) and the actual origin\n>> \n>> file, blob, cmdline (hmm, maybe cmd-line? ;-) )\n> \n> If we are going to spell it out, \"command-line\" would be the way to\n> go. \"cmdline\" is probably OK.\n\nI think cmdline is OK, too. However, if the list thinks that there is a chance\nthat it could be incomprehensible to the user then I would prefer \"command-line\".\n\n- Lars"},{"id":"278293","messageId":"51832840-B879-4650-9DC5-E15EAA9919B9@gmail.com","threadId":"41413","inReplyTo":"56C253B8.1070702@ramsayjones.plus.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-16T09:51:50Z","receivedAt":"2016-02-16T09:51:50Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Feb 2016, at 23:39, Ramsay Jones wrote:\n\n> \n> \n> On 15/02/16 21:40, Jeff King wrote:\n>> On Mon, Feb 15, 2016 at 09:36:23PM +0000, Ramsay Jones wrote:\n>> \n>>>> +test_expect_success '--show-origin stdin' '\n>>>> +\tcat >expect <<-\\EOF &&\n>>>> +\t\tstdin:\tuser.custom=true\n>>> \n>>> So, as with the previous patch, I think this should be:\n>>> \t\tfile:\tuser.custom=true\n>> \n>> That's ambiguous with a file named \"\", which was the point of\n>> having the two separate prefixes in the first place.\n>> \n>> I think in practice we _could_ get by with an ambiguous output (it's not\n>> like \"\" is a common filename), but that was discussed earlier in\n>> the thread, and Lars decided to go for something unambiguous.\n> \n> sure, I just don't think it would cause a problem in practice.\n> How about using '-' for ? Hmm, you can actually create\n> such a file in the filesystem! Oh well, I guess its not a big deal.\n> \n>> \n>> That doesn't necessarily have to bleed over into the error messages,\n>> though (which could continue to use \"\" if we want to put in a\n>> little extra code to covering the cases separately.\n> \n> Yep.\nOK, I am happy to add the extra code. However, out of curiosity, can\nyou explain in what cases you actually use configs from stdin? I wasn't\naware of this feature before working on this patch and I still wonder\nwhen I would use it. If it is only a seldom used feature then I am not\nsure if adding the extra code to restore the existing error message\nis worth the effort?\n\nThanks,\nLars\n"},{"id":"278294","messageId":"AC15F01E-4B2C-4E38-98D7-F2D5B139B488@gmail.com","threadId":"41413","inReplyTo":"xmqq4md9o7da.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-16T09:52:57Z","receivedAt":"2016-02-16T09:52:57Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 16 Feb 2016, at 00:56, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n>>> An existing test t3300 tells me that a test that uses a path with a\n>>> tab needs to be skipped on FAT/NTFS. If your goal is to make sure\n>>> dquote is exercised, can't we just do with a path with a SP in it or\n>>> something?\n>> \n>> It has to trigger quote_c_style(). You can see the complete set of\n>> quoted characters in quote.c:sq_lookup, but space is not one of them.\n>> Probably double-quote or backslash is the best bet, as the rest are all\n>> control characters.\n> \n> Yeah, 3300 seems to use dq for that.\n> \n> Thanks for checking.\n\nThanks for the pointer Junio. I didn't think of FAT/NTFS. I will mimic\n3300 and use double quotes.\n\nThanks,\nLars\n"},{"id":"278363","messageId":"56C3524F.3000504@ramsayjones.plus.com","threadId":"41413","inReplyTo":"51832840-B879-4650-9DC5-E15EAA9919B9@gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-16T16:46:07Z","receivedAt":"2016-02-16T16:46:07Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 16/02/16 09:51, Lars Schneider wrote:\n> \n> On 15 Feb 2016, at 23:39, Ramsay Jones wrote:\n> \n>>\n>>\n>> On 15/02/16 21:40, Jeff King wrote:\n>>> On Mon, Feb 15, 2016 at 09:36:23PM +0000, Ramsay Jones wrote:\n>>>\n>>>>> +test_expect_success '--show-origin stdin' '\n>>>>> +\tcat >expect <<-\\EOF &&\n>>>>> +\t\tstdin:\tuser.custom=true\n>>>>\n>>>> So, as with the previous patch, I think this should be:\n>>>> \t\tfile:\tuser.custom=true\n>>>\n>>> That's ambiguous with a file named \"\", which was the point of\n>>> having the two separate prefixes in the first place.\n>>>\n>>> I think in practice we _could_ get by with an ambiguous output (it's not\n>>> like \"\" is a common filename), but that was discussed earlier in\n>>> the thread, and Lars decided to go for something unambiguous.\n>>\n>> sure, I just don't think it would cause a problem in practice.\n>> How about using '-' for ? Hmm, you can actually create\n>> such a file in the filesystem! Oh well, I guess its not a big deal.\n>>\n>>>\n>>> That doesn't necessarily have to bleed over into the error messages,\n>>> though (which could continue to use \"\" if we want to put in a\n>>> little extra code to covering the cases separately.\n>>\n>> Yep.\n\nSorry for not replying earlier - today has been hectic, so far.\n\n> OK, I am happy to add the extra code. \n\nUnless I've missed something (quite possible), this patch does not\nneed to change. (you have (both) convinced me that your current\nsolution is the best).\n\nThe only change that I would suggest is the one-liner I already\nsuggested to the previous patch (plus the one-liner in the test,\nof course. err ... so the two-liner!). Having said that, I didn't\ntry it out - I was just typing into my email client, so ...\n\n> However, out of curiosity, can\n> you explain in what cases you actually use configs from stdin? I wasn't\n> aware of this feature before working on this patch and I still wonder\n> when I would use it.\n\nPersonally, I can't imagine ever using it. (I don't have a great\nimagination. ;-)\n\nSince I couldn't recall when this feature was added, I looked for\nthe commit that added it and found it was merged in commit 08f36302.\nIn particular, commit 3caec73b (\"config: teach \"git config --file -\"\nto read from the standard input\", 19-02-2014) does not seem to\ninclude any motivation for the change. The corresponding release\nnotes for v2.2.0 do not seem to add anything either.\n\nSo, I'm not much help here. :(\n\n[Ah, looking at the date on the merge explains why I didn't\nnotice this.]\n\n> If it is only a seldom used feature then I am not\n> sure if adding the extra code to restore the existing error message\n> is worth the effort?\n\nATB,\nRamsay Jones\n"},{"id":"278365","messageId":"20160216173853.GA15026@sigill.intra.peff.net","threadId":"41413","inReplyTo":"56C3524F.3000504@ramsayjones.plus.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-16T17:38:54Z","receivedAt":"2016-02-16T17:38:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 16, 2016 at 04:46:07PM +0000, Ramsay Jones wrote:\n\n> > OK, I am happy to add the extra code. \n> \n> Unless I've missed something (quite possible), this patch does not\n> need to change. (you have (both) convinced me that your current\n> solution is the best).\n> \n> The only change that I would suggest is the one-liner I already\n> suggested to the previous patch (plus the one-liner in the test,\n> of course. err ... so the two-liner!). Having said that, I didn't\n> try it out - I was just typing into my email client, so ...\n\nI think it's more than that one-liner. This patch shows \"type:name\"\nverbatim from what is passed into do_config_from_file, as does the error\nmessage. If they are going to have different output formats (e.g.,\n\"\" versus \"stdin\"), there needs to be logic transforming them in\nat least one of the spots.\n\n-Peff\n"},{"id":"278396","messageId":"56C39D8B.3060700@ramsayjones.plus.com","threadId":"41413","inReplyTo":"20160215174218.GB26443@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-16T22:07:07Z","receivedAt":"2016-02-16T22:07:07Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 15/02/16 17:42, Jeff King wrote:\n> On Mon, Feb 15, 2016 at 11:17:45AM +0100, larsxschneider@gmail.com wrote:\n> \n>> From: Lars Schneider \n>>\n>> Use the config type to print more detailed error messages that inform\n>> the user about the origin of a config error (file, stdin, blob).\n>>\n>> Signed-off-by: Lars Schneider \n>> ---\n>> cache.h | 6 ++++--\n>> config.c | 36 +++++++++++++++++++++++++-----------\n>> submodule-config.c | 4 ++--\n>> t/t1300-repo-config.sh | 8 +++++++-\n>> t/t1308-config-set.sh | 4 ++--\n>> 5 files changed, 40 insertions(+), 18 deletions(-)\n> \n> Looks good to me.\n> \n> -Peff\n> \n\nHi Lars,\n\nCould you please squash this into your patch.\n\nThanks!\n\nATB,\nRamsay Jones\n\n-- >8 --\nFrom: Ramsay Jones \nDate: Tue, 16 Feb 2016 21:35:41 +0000\nSubject: [PATCH] config: avoid declaration missing prototype warnings\n\nSigned-off-by: Ramsay Jones \n---\n cache.h | 4 ++--\n config.c | 4 ++--\n 2 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 3ff94dc..7f1fa65 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1546,8 +1546,8 @@ extern const char *get_log_output_encoding(void);\n extern const char *get_commit_output_encoding(void);\n \n extern int git_config_parse_parameter(const char *, config_fn_t fn, void *data);\n-extern const char *current_config_type();\n-extern const char *current_config_name();\n+extern const char *current_config_type(void);\n+extern const char *current_config_name(void);\n \n struct config_include_data {\n \tint depth;\ndiff --git a/config.c b/config.c\nindex f1efba8..0a35323 100644\n--- a/config.c\n+++ b/config.c\n@@ -2414,12 +2414,12 @@ int parse_config_key(const char *var,\n \treturn 0;\n }\n \n-const char *current_config_type()\n+const char *current_config_type(void)\n {\n \treturn cf && cf->type ? cf->type : \"cmdline\";\n }\n \n-const char *current_config_name()\n+const char *current_config_name(void)\n {\n \treturn cf && cf->name ? cf->name : \"\";\n }\n-- \n2.7.0\n"},{"id":"278398","messageId":"56C39F55.1090903@ramsayjones.plus.com","threadId":"41413","inReplyTo":"20160216173853.GA15026@sigill.intra.peff.net","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2016-02-16T22:14:45Z","receivedAt":"2016-02-16T22:14:45Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\n\nOn 16/02/16 17:38, Jeff King wrote:\n> On Tue, Feb 16, 2016 at 04:46:07PM +0000, Ramsay Jones wrote:\n> \n>>> OK, I am happy to add the extra code. \n>>\n>> Unless I've missed something (quite possible), this patch does not\n>> need to change. (you have (both) convinced me that your current\n>> solution is the best).\n>>\n>> The only change that I would suggest is the one-liner I already\n>> suggested to the previous patch (plus the one-liner in the test,\n>> of course. err ... so the two-liner!). Having said that, I didn't\n>> try it out - I was just typing into my email client, so ...\n> \n> I think it's more than that one-liner. This patch shows \"type:name\"\n> verbatim from what is passed into do_config_from_file, as does the error\n> message. If they are going to have different output formats (e.g.,\n> \"\" versus \"stdin\"), there needs to be logic transforming them in\n> at least one of the spots.\n\nUgh, yes you are right.\n\nHmm, I just hacked something up (see below) and, since its a bit\nugly, I'm now in two minds! (it could be improved, of course). ;-)\n\nSo, I'll leave it to yourself and Lars to decide.\n\nATB,\nRamsay Jones\n\n-- >8 --\nFrom: Ramsay Jones \nDate: Tue, 16 Feb 2016 21:39:47 +0000\nSubject: [PATCH] config: fixup '' error messages\n\nSigned-off-by: Ramsay Jones \n---\n config.c | 22 ++++++++++++++++++----\n t/t1300-repo-config.sh | 2 +-\n 2 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 0a35323..adc808c 100644\n--- a/config.c\n+++ b/config.c\n@@ -417,6 +417,8 @@ static int git_parse_source(config_fn_t fn, void *data)\n \tint comment = 0;\n \tint baselen = 0;\n \tstruct strbuf *var = &cf->var;\n+\tconst char *cftype = cf->type;\n+\tconst char *cfname = cf->name;\n \n \t/* U+FEFF Byte Order Mark in UTF8 */\n \tconst char *bomptr = utf8_bom;\n@@ -471,10 +473,14 @@ static int git_parse_source(config_fn_t fn, void *data)\n \t\tif (get_value(fn, data, var) < 0)\n \t\t\tbreak;\n \t}\n+\tif (!strcmp(cftype, \"stdin\")) {\n+\t\tcftype = \"file\";\n+\t\tcfname = \"\";\n+\t}\n \tif (cf->die_on_error)\n-\t\tdie(_(\"bad config line %d in %s %s\"), cf->linenr, cf->type, cf->name);\n+\t\tdie(_(\"bad config line %d in %s %s\"), cf->linenr, cftype, cfname);\n \telse\n-\t\treturn error(_(\"bad config line %d in %s %s\"), cf->linenr, cf->type, cf->name);\n+\t\treturn error(_(\"bad config line %d in %s %s\"), cf->linenr, cftype, cfname);\n }\n \n static int parse_unit_factor(const char *end, uintmax_t *val)\n@@ -589,9 +595,17 @@ static void die_bad_number(const char *name, const char *value)\n \tif (!value)\n \t\tvalue = \"\";\n \n-\tif (cf && cf->type && cf->name)\n+\tif (cf && cf->type && cf->name) {\n+\t\tconst char *cftype = cf->type;\n+\t\tconst char *cfname = cf->name;\n+\n+\t\tif (!strcmp(cftype, \"stdin\")) {\n+\t\t\tcftype = \"file\";\n+\t\t\tcfname = \"\";\n+\t\t}\n \t\tdie(_(\"bad numeric config value '%s' for '%s' in %s %s: %s\"),\n-\t\t value, name, cf->type, cf->name, reason);\n+\t\t value, name, cftype, cfname, reason);\n+\t}\n \tdie(_(\"bad numeric config value '%s' for '%s': %s\"), value, name, reason);\n }\n \ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 4abbdf9..4497688 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -707,7 +707,7 @@ test_expect_success 'invalid unit' '\n '\n \n test_expect_success 'invalid stdin config' '\n-\techo \"fatal: bad config line 1 in stdin \" >expect &&\n+\techo \"fatal: bad config line 1 in file \" >expect &&\n \techo \"[broken\" | test_must_fail git config --list --file - >output 2>&1 &&\n \ttest_cmp expect output\n '\n-- \n2.7.0\n"},{"id":"278399","messageId":"20160216221739.GA8370@sigill.intra.peff.net","threadId":"41413","inReplyTo":"56C39F55.1090903@ramsayjones.plus.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-02-16T22:17:39Z","receivedAt":"2016-02-16T22:17:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 16, 2016 at 10:14:45PM +0000, Ramsay Jones wrote:\n\n> > I think it's more than that one-liner. This patch shows \"type:name\"\n> > verbatim from what is passed into do_config_from_file, as does the error\n> > message. If they are going to have different output formats (e.g.,\n> > \"\" versus \"stdin\"), there needs to be logic transforming them in\n> > at least one of the spots.\n> \n> Ugh, yes you are right.\n> \n> Hmm, I just hacked something up (see below) and, since its a bit\n> ugly, I'm now in two minds! (it could be improved, of course). ;-)\n> \n> So, I'll leave it to yourself and Lars to decide.\n> [...]\n> +\tif (!strcmp(cftype, \"stdin\")) {\n> +\t\tcftype = \"file\";\n> +\t\tcfname = \"\";\n> +\t}\n\nI think if we go this route it would be cleaner to just make \"type\" an\nenum and convert it to the appropriate string in the callers. But other\nthan that, I think your patch is along the correct lines.\n\nI dunno. Personally I am fine with the change in error messages done by\nLars. I could go either way.\n\n-Peff\n\nPS Thanks also for your patch fixing the prototypes. I completely missed\n that.\n"},{"id":"278440","messageId":"DC5B6F35-CBB5-4DC4-A875-275BF072AD91@gmail.com","threadId":"41413","inReplyTo":"CAPig+cS=TtVGTxo0oHny4GpAAaej93T2kP93upfHtPMkAd7V0g@mail.gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-17T08:32:25Z","receivedAt":"2016-02-17T08:32:25Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 15 Feb 2016, at 21:58, Eric Sunshine wrote:\n\n> On Mon, Feb 15, 2016 at 5:17 AM, wrote:\n>> If config values are queried using 'git config' (e.g. via --get,\n>> --get-all, --get-regexp, or --list flag) then it is sometimes hard to\n>> find the configuration file where the values were defined.\n>> \n>> Teach 'git config' the '--show-origin' option to print the source\n>> configuration file for every printed value.\n>> \n>> Based-on-patch-by: Jeff King \n>> Signed-off-by: Lars Schneider \n>> ---\n>> diff --git a/builtin/config.c b/builtin/config.c\n>> @@ -27,6 +28,7 @@ static int actions, types;\n>> static const char *get_color_slot, *get_colorbool_slot;\n>> static int end_null;\n> \n> Not related to your changes, but I just realized that this variable\n> really ought to be named 'end_nul' since we're talking about the\n> character NUL, not a NULL pointer.\n> \n>> static int respect_includes = -1;\n>> +static int show_origin;\n>> @@ -81,6 +83,7 @@ static struct option builtin_config_options[] = {\n>> OPT_BOOL('z', \"null\", &end_null, N_(\"terminate values with NUL byte\")),\n> \n> Likewise, the long option name should be --nul rather than --null, or\n> the long name could be dropped altogether since some other commands\n> just recognize short option -z.\n> \n> There is no need for this patch series to address this anomaly; it's\n> perhaps low-hanging fruit for someone wanting to join the project. The\n> only very minor wrinkle is that we'd still need to recognize --null as\n> a deprecated (and undocumented) alias for --nul.\n\nDoes the list have a place to document these ideas for newbies to be found?\n\nThanks,\nLars"},{"id":"278452","messageId":"alpine.DEB.2.20.1602171412080.6516@virtualbox","threadId":"41413","inReplyTo":"1455531466-16617-3-git-send-email-larsxschneider@gmail.com","subject":"Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-02-17T13:12:44Z","receivedAt":"2016-02-17T13:12:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Lars,\n\nOn Mon, 15 Feb 2016, larsxschneider@gmail.com wrote:\n\n> From: Lars Schneider \n> \n> Use the config type to print more detailed error messages that inform\n> the user about the origin of a config error (file, stdin, blob).\n\nGiven that you settled on `--show-origin` as the command-line option, I\nwould have expected s/type/origin/g, too...\n\nCiao,\nDscho\n"},{"id":"278465","messageId":"xmqqmvqzjnpm.fsf@gitster.mtv.corp.google.com","threadId":"41413","inReplyTo":"DC5B6F35-CBB5-4DC4-A875-275BF072AD91@gmail.com","subject":"Re: [PATCH v4 3/3] config: add '--show-origin' option to print the origin of a config value","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-02-17T16:39:17Z","receivedAt":"2016-02-17T16:39:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Lars Schneider writes:\n\n>> There is no need for this patch series to address this anomaly; it's\n>> perhaps low-hanging fruit for someone wanting to join the project. The\n>> only very minor wrinkle is that we'd still need to recognize --null as\n>> a deprecated (and undocumented) alias for --nul.\n>\n> Does the list have a place to document these ideas for newbies to be found?\n\nI do not know of one offhand, but if somebody (or like-minded group\nof people) starts to collect one, I am sure it would be very much\nappreciated.\n\nA search for \"low hanging fruit\" and \"hint hint\" in the list archive\nused to work well as a first-order approximation; using these\nkeywords takes conscious effort on those who write them, though ;-).\n"},{"id":"278556","messageId":"46CB7F8C-2A0C-41E1-A6B7-C26448CAE052@gmail.com","threadId":"41413","inReplyTo":"alpine.DEB.2.20.1602171412080.6516@virtualbox","subject":"Re: [PATCH v4 2/3] config: add 'type' to config_source struct that identifies config type","fromName":"Lars Schneider","fromEmail":"larsxschneider@gmail.com","sentAt":"2016-02-18T08:46:36Z","receivedAt":"2016-02-18T08:46:36Z","isPatch":true,"sender":{"key":"larsxschneider@gmail.com","avatar":"https://avatars.githubusercontent.com/u/477434?v=4"},"body":"\nOn 17 Feb 2016, at 14:12, Johannes Schindelin wrote:\n\n> Hi Lars,\n> \n> On Mon, 15 Feb 2016, larsxschneider@gmail.com wrote:\n> \n>> From: Lars Schneider \n>> \n>> Use the config type to print more detailed error messages that inform\n>> the user about the origin of a config error (file, stdin, blob).\n> \n> Given that you settled on `--show-origin` as the command-line option, I\n> would have expected s/type/origin/g, too...\n\nAgreed. Junio suggested \"origin_type\" which I like, too.\n\nThanks,\nLars"}]}