{"thread":{"id":"48361","subject":"[PATCH 0/5] builtin/config.c: combined series '--type', '--default'","startedAt":"2018-04-26T04:25:33Z","lastAt":"2018-04-27T06:22:27Z","messageCount":20,"participants":["Taylor Blau","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"345858","messageId":"cover.1524716420.git.me@ttaylorr.com","threadId":"48361","inReplyTo":null,"subject":"[PATCH 0/5] builtin/config.c: combined series '--type', '--default'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T04:25:17Z","receivedAt":"2018-04-26T04:25:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nAttached is a combination of my two series to add [--type=] and\n[--default=] to 'git-config(1)'.\n\nI have not changed anything in these patches since their last review in\n[1], [2], other than to:\n\n * Ensure that they merge cleanly into 'master' and,\n\n * Incorporate the patch that I sent to Junio in [3], to remove any\n decl-after-statement's.\n\nMy intention is that these patches will be easier to queue together and\nat the same time, rather than individually. This was suggested to me by\nEric in [4].\n\n\nThanks,\nTaylor\n\n[1]: https://public-inbox.org/git/xmqq8t9jgbe1.fsf@gitster-ct.c.googlers.com\n[2]: https://public-inbox.org/git/xmqqk1tf4yhl.fsf@gitster-ct.c.googlers.com\n[3]: https://public-inbox.org/git/20180419030142.GA28273@syl.local\n[4]: https://public-inbox.org/git/CAPig+cSr744Y293qvgLG8jLHdNsGypkHU6QUQ-AcOyk=-JAbDw@mail.gmail.com\n\nTaylor Blau (5):\n builtin/config.c: treat type specifiers singularly\n builtin/config.c: support `--type=` as preferred alias for\n `--type`\n builtin/config: introduce `--default`\n config.c: introduce 'git_config_color' to parse ANSI colors\n builtin/config: introduce `color` type specifier\n\n Documentation/git-config.txt | 81 ++++++++++++--------\n builtin/config.c | 143 ++++++++++++++++++++++++++++-------\n config.c | 10 +++\n config.h | 1 +\n t/t1300-repo-config.sh | 93 +++++++++++++++++++++++\n t/t1310-config-default.sh | 36 +++++++++\n 6 files changed, 305 insertions(+), 59 deletions(-)\n create mode 100755 t/t1310-config-default.sh\n\n--\n2.17.0\n"},{"id":"345859","messageId":"8d8bda20d2df7d59cacfe611b148dbdb33f6a978.1524716420.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524716420.git.me@ttaylorr.com","subject":"[PATCH 1/5] builtin/config.c: treat type specifiers singularly","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T04:25:20Z","receivedAt":"2018-04-26T04:25:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Internally, we represent `git config`'s type specifiers as a bitset\nusing OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique\nallows for the representation of multiple type specifiers in the `int\ntypes` field, but this multi-representation is left unused.\n\nIn fact, `git config` will not accept multiple type specifiers at a\ntime, as indicated by:\n\n $ git config --int --bool some.section\n error: only one type at a time.\n\nThis patch uses `OPT_SET_INT` to prefer the _last_ mentioned type\nspecifier, so that the above command would instead be valid, and a\nsynonym of:\n\n $ git config --bool some.section\n\nThis change is motivated by two urges: (1) it does not make sense to\nrepresent a singular type specifier internally as a bitset, only to\ncomplain when there are multiple bits in the set. `OPT_SET_INT` is more\nwell-suited to this task than `OPT_BIT` is. (2) a future patch will\nintroduce `--type=`, and we would like not to complain in the\nfollowing situation:\n\n $ git config --int --type=int\n\nSigned-off-by: Taylor Blau \n---\n builtin/config.c | 49 +++++++++++++++++++-----------------------\n t/t1300-repo-config.sh | 11 ++++++++++\n 2 files changed, 33 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 01169dd628..92fb8d56b1 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -25,7 +25,7 @@ static char term = '\\n';\n \n static int use_global_config, use_system_config, use_local_config;\n static struct git_config_source given_config_source;\n-static int actions, types;\n+static int actions, type;\n static int end_null;\n static int respect_includes_opt = -1;\n static struct config_options config_options;\n@@ -55,11 +55,11 @@ static int show_origin;\n #define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \\\n \t\t\tACTION_GET_REGEXP | ACTION_GET_URLMATCH)\n \n-#define TYPE_BOOL (1<<0)\n-#define TYPE_INT (1<<1)\n-#define TYPE_BOOL_OR_INT (1<<2)\n-#define TYPE_PATH (1<<3)\n-#define TYPE_EXPIRY_DATE (1<<4)\n+#define TYPE_BOOL\t\t1\n+#define TYPE_INT\t\t2\n+#define TYPE_BOOL_OR_INT\t3\n+#define TYPE_PATH\t\t4\n+#define TYPE_EXPIRY_DATE\t5\n \n static struct option builtin_config_options[] = {\n \tOPT_GROUP(N_(\"Config file location\")),\n@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {\n \tOPT_BIT(0, \"get-color\", &actions, N_(\"find the color configured: slot [default]\"), ACTION_GET_COLOR),\n \tOPT_BIT(0, \"get-colorbool\", &actions, N_(\"find the color setting: slot [stdout-is-tty]\"), ACTION_GET_COLORBOOL),\n \tOPT_GROUP(N_(\"Type\")),\n-\tOPT_BIT(0, \"bool\", &types, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n-\tOPT_BIT(0, \"int\", &types, N_(\"value is decimal number\"), TYPE_INT),\n-\tOPT_BIT(0, \"bool-or-int\", &types, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n-\tOPT_BIT(0, \"path\", &types, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n-\tOPT_BIT(0, \"expiry-date\", &types, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n+\tOPT_SET_INT(0, \"bool\", &type, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n+\tOPT_SET_INT(0, \"int\", &type, N_(\"value is decimal number\"), TYPE_INT),\n+\tOPT_SET_INT(0, \"bool-or-int\", &type, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n+\tOPT_SET_INT(0, \"path\", &type, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n+\tOPT_SET_INT(0, \"expiry-date\", &type, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n \tOPT_GROUP(N_(\"Other\")),\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@@ -149,26 +149,26 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value\n \t\tif (show_keys)\n \t\t\tstrbuf_addch(buf, key_delim);\n \n-\t\tif (types == TYPE_INT)\n+\t\tif (type == TYPE_INT)\n \t\t\tstrbuf_addf(buf, \"%\"PRId64,\n \t\t\t\t git_config_int64(key_, value_ ? value_ : \"\"));\n-\t\telse if (types == TYPE_BOOL)\n+\t\telse if (type == TYPE_BOOL)\n \t\t\tstrbuf_addstr(buf, git_config_bool(key_, value_) ?\n \t\t\t\t \"true\" : \"false\");\n-\t\telse if (types == TYPE_BOOL_OR_INT) {\n+\t\telse if (type == TYPE_BOOL_OR_INT) {\n \t\t\tint is_bool, v;\n \t\t\tv = git_config_bool_or_int(key_, value_, &is_bool);\n \t\t\tif (is_bool)\n \t\t\t\tstrbuf_addstr(buf, v ? \"true\" : \"false\");\n \t\t\telse\n \t\t\t\tstrbuf_addf(buf, \"%d\", v);\n-\t\t} else if (types == TYPE_PATH) {\n+\t\t} else if (type == TYPE_PATH) {\n \t\t\tconst char *v;\n \t\t\tif (git_config_pathname(&v, key_, value_) < 0)\n \t\t\t\treturn -1;\n \t\t\tstrbuf_addstr(buf, v);\n \t\t\tfree((char *)v);\n-\t\t} else if (types == TYPE_EXPIRY_DATE) {\n+\t\t} else if (type == TYPE_EXPIRY_DATE) {\n \t\t\ttimestamp_t t;\n \t\t\tif (git_config_expiry_date(&t, key_, value_) < 0)\n \t\t\t\treturn -1;\n@@ -287,7 +287,7 @@ static char *normalize_value(const char *key, const char *value)\n \tif (!value)\n \t\treturn NULL;\n \n-\tif (types == 0 || types == TYPE_PATH || types == TYPE_EXPIRY_DATE)\n+\tif (type == 0 || type == TYPE_PATH || type == TYPE_EXPIRY_DATE)\n \t\t/*\n \t\t * We don't do normalization for TYPE_PATH here: If\n \t\t * the path is like ~/foobar/, we prefer to store\n@@ -296,11 +296,11 @@ static char *normalize_value(const char *key, const char *value)\n \t\t * Also don't do normalization for expiry dates.\n \t\t */\n \t\treturn xstrdup(value);\n-\tif (types == TYPE_INT)\n+\tif (type == TYPE_INT)\n \t\treturn xstrfmt(\"%\"PRId64, git_config_int64(key, value));\n-\tif (types == TYPE_BOOL)\n+\tif (type == TYPE_BOOL)\n \t\treturn xstrdup(git_config_bool(key, value) ? \"true\" : \"false\");\n-\tif (types == TYPE_BOOL_OR_INT) {\n+\tif (type == TYPE_BOOL_OR_INT) {\n \t\tint is_bool, v;\n \t\tv = git_config_bool_or_int(key, value, &is_bool);\n \t\tif (!is_bool)\n@@ -309,7 +309,7 @@ static char *normalize_value(const char *key, const char *value)\n \t\t\treturn xstrdup(v ? \"true\" : \"false\");\n \t}\n \n-\tdie(\"BUG: cannot normalize type %d\", types);\n+\tdie(\"BUG: cannot normalize type %d\", type);\n }\n \n static int get_color_found;\n@@ -566,12 +566,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tkey_delim = '\\n';\n \t}\n \n-\tif (HAS_MULTI_BITS(types)) {\n-\t\terror(\"only one type at a time.\");\n-\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n-\t}\n-\n-\tif ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && types) {\n+\tif ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {\n \t\terror(\"--get-color and variable type are incoherent\");\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex e95b1e67da..ed9b9cd344 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1611,4 +1611,15 @@ test_expect_success '--local requires a repo' '\n \ttest_expect_code 128 nongit git config --local foo.bar\n '\n \n+cat >.git/config <<-\\EOF &&\n+[core]\n+number = 10\n+EOF\n+\n+test_expect_success 'later legacy specifiers are given precedence' '\n+\tgit config --bool --int core.number >actual &&\n+\techo 10 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.17.0\n\n"},{"id":"345861","messageId":"1bc42a611089a2552b1d8bc83ff229be0a5136ff.1524716420.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524716420.git.me@ttaylorr.com","subject":"[PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T04:25:22Z","receivedAt":"2018-04-26T04:25:33Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"`git config` has long allowed the ability for callers to provide a 'type\nspecifier', which instructs `git config` to (1) ensure that incoming\nvalues can be interpreted as that type, and (2) that outgoing values are\ncanonicalized under that type.\n\nIn another series, we propose to extend this functionality with\n`--type=color` and `--default` to replace `--get-color`.\n\nHowever, we traditionally use `--color` to mean \"colorize this output\",\ninstead of \"this value should be treated as a color\".\n\nCurrently, `git config` does not support this kind of colorization, but\nwe should be careful to avoid squatting on this option too soon, so that\n`git config` can support `--color` (in the traditional sense) in the\nfuture, if that is desired.\n\nIn this patch, we support `--type=` in\naddition to `--int`, `--bool`, and etc. This allows the aforementioned\nupcoming patch to support querying a color value with a default via\n`--type=color --default=...`, without squandering `--color`.\n\nWe retain the historic behavior of complaining when multiple,\nlegacy-style `--` flags are given, as well as extend this to\nconflicting new-style `--type=` flags. `--int --type=int` (and its\ncommutative pair) does not complain, but `--bool --type=int` (and its\ncommutative pair) does.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-config.txt | 71 ++++++++++++++++++++----------------\n builtin/config.c | 66 ++++++++++++++++++++++++++++++---\n t/t1300-repo-config.sh | 58 +++++++++++++++++++++++++++--\n 3 files changed, 155 insertions(+), 40 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex e09ed5d7d5..b759009c75 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -9,13 +9,13 @@ git-config - Get and set repository or global options\n SYNOPSIS\n --------\n [verse]\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] [--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' [] [--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=] [--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@@ -38,12 +38,10 @@ existing values that match the regexp are updated or unset. If\n you want to handle the lines that do *not* match the regex, just\n prepend a single exclamation mark in front (see also <>).\n \n-The type specifier can be either `--int` or `--bool`, to make\n-'git config' ensure that the variable(s) are of the given type and\n-convert the value to the canonical form (simple decimal number for int,\n-a \"true\" or \"false\" string for bool), or `--path`, which does some\n-path expansion (see `--path` below). If no type specifier is passed, no\n-checks or transformations are performed on the value.\n+The `--type=` option instructs 'git config' to ensure that incoming and\n+outgoing values are canonicalize-able under the given . If no\n+`--type=` is given, no canonicalization will be performed. Callers may\n+unset an existing `--type` specifier with `--no-type`.\n \n When reading, the values are read from the system, global and\n repository local configuration files by default, and options\n@@ -160,30 +158,39 @@ See also <>.\n --list::\n \tList all variables set in config file, along with their values.\n \n---bool::\n-\t'git config' will ensure that the output is \"true\" or \"false\"\n+--type ::\n+ 'git config' will ensure that any input or output is valid under the given\n+ type constraint(s), and will canonicalize outgoing values in ``'s\n+ canonical form.\n++\n+Valid ``'s include:\n++\n+- 'bool': canonicalize values as either \"true\" or \"false\".\n+- 'int': canonicalize values as simple decimal numbers. An optional suffix of\n+ 'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or\n+ 1073741824 upon input.\n+- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described\n+ above.\n+- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and\n+ `~user` to the home directory for the specified user. This specifier has no\n+ effect when setting the value (but you can use `git config section.variable\n+ ~/` from the command line to let your shell do the expansion.)\n+- 'expiry-date': canonicalize by converting from a fixed or relative date-string\n+ to a timestamp. This specifier has no effect when setting the value.\n++\n \n+--bool::\n --int::\n-\t'git config' will ensure that the output is a simple\n-\tdecimal number. An optional value suffix of 'k', 'm', or 'g'\n-\tin the config file will cause the value to be multiplied\n-\tby 1024, 1048576, or 1073741824 prior to output.\n-\n --bool-or-int::\n-\t'git config' will ensure that the output matches the format of\n-\teither --bool or --int, as described above.\n-\n --path::\n-\t`git config` will expand a leading `~` to the value of\n-\t`$HOME`, and `~user` to the home directory for the\n-\tspecified user. This option has no effect when setting the\n-\tvalue (but you can use `git config section.variable ~/`\n-\tfrom the command line to let your shell do the expansion).\n-\n --expiry-date::\n-\t`git config` will ensure that the output is converted from\n-\ta fixed or relative date-string to a timestamp. This option\n-\thas no effect when setting the value.\n+ Historical options for selecting a type specifier. Prefer instead `--type`,\n+ (see: above).\n+\n+--no-type::\n+ Un-sets the previously set type specifier (if one was previously set). This\n+ option requests that 'git config' not canonicalize the retrieved variable.\n+ `--no-type` has no effect without `--type=` or `--`.\n \n -z::\n --null::\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 92fb8d56b1..2f91ef15a4 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -61,6 +61,61 @@ static int show_origin;\n #define TYPE_PATH\t\t4\n #define TYPE_EXPIRY_DATE\t5\n \n+#define OPT_CALLBACK_VALUE(s, l, v, h, i) \\\n+\t{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \\\n+\tPARSE_OPT_NONEG, option_parse_type, (i) }\n+\n+static struct option builtin_config_options[];\n+\n+static int option_parse_type(const struct option *opt, const char *arg,\n+\t\t\t int unset)\n+{\n+\tint new_type;\n+\tint *to_type;\n+\n+\tif (unset) {\n+\t\t*((int *) opt->value) = 0;\n+\t\treturn 0;\n+\t}\n+\n+\t/*\n+\t * To support '--' style flags, begin with new_type equal to\n+\t * opt->defval.\n+\t */\n+\tnew_type = opt->defval;\n+\tif (!new_type) {\n+\t\tif (!strcmp(arg, \"bool\"))\n+\t\t\tnew_type = TYPE_BOOL;\n+\t\telse if (!strcmp(arg, \"int\"))\n+\t\t\tnew_type = TYPE_INT;\n+\t\telse if (!strcmp(arg, \"bool-or-int\"))\n+\t\t\tnew_type = TYPE_BOOL_OR_INT;\n+\t\telse if (!strcmp(arg, \"path\"))\n+\t\t\tnew_type = TYPE_PATH;\n+\t\telse if (!strcmp(arg, \"expiry-date\"))\n+\t\t\tnew_type = TYPE_EXPIRY_DATE;\n+\t\telse\n+\t\t\tdie(_(\"unrecognized --type argument, %s\"), arg);\n+\t}\n+\n+\t*to_type = opt->value;\n+\tif (*to_type && *to_type != new_type) {\n+\t\t/*\n+\t\t * Complain when there is a new type not equal to the old type.\n+\t\t * This allows for combinations like '--int --type=int' and\n+\t\t * '--type=int --type=int', but disallows ones like '--type=bool\n+\t\t * --int' and '--type=bool\n+\t\t * --type=int'.\n+\t\t */\n+\t\terror(\"only one type at a time.\");\n+\t\tusage_with_options(builtin_config_usage,\n+\t\t\tbuiltin_config_options);\n+\t}\n+\t*to_type = new_type;\n+\n+\treturn 0;\n+}\n+\n static struct option builtin_config_options[] = {\n \tOPT_GROUP(N_(\"Config file location\")),\n \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n@@ -84,11 +139,12 @@ static struct option builtin_config_options[] = {\n \tOPT_BIT(0, \"get-color\", &actions, N_(\"find the color configured: slot [default]\"), ACTION_GET_COLOR),\n \tOPT_BIT(0, \"get-colorbool\", &actions, N_(\"find the color setting: slot [stdout-is-tty]\"), ACTION_GET_COLORBOOL),\n \tOPT_GROUP(N_(\"Type\")),\n-\tOPT_SET_INT(0, \"bool\", &type, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n-\tOPT_SET_INT(0, \"int\", &type, N_(\"value is decimal number\"), TYPE_INT),\n-\tOPT_SET_INT(0, \"bool-or-int\", &type, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n-\tOPT_SET_INT(0, \"path\", &type, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n-\tOPT_SET_INT(0, \"expiry-date\", &type, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n+\tOPT_CALLBACK('t', \"type\", &type, \"\", N_(\"value is given this type\"), option_parse_type),\n+\tOPT_CALLBACK_VALUE(0, \"bool\", &type, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n+\tOPT_CALLBACK_VALUE(0, \"int\", &type, N_(\"value is decimal number\"), TYPE_INT),\n+\tOPT_CALLBACK_VALUE(0, \"bool-or-int\", &type, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n+\tOPT_CALLBACK_VALUE(0, \"path\", &type, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n+\tOPT_CALLBACK_VALUE(0, \"expiry-date\", &type, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n \tOPT_GROUP(N_(\"Other\")),\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\")),\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex ed9b9cd344..1e3a1ace14 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1613,13 +1613,65 @@ test_expect_success '--local requires a repo' '\n \n cat >.git/config <<-\\EOF &&\n [core]\n+foo = true\n number = 10\n+big = 1M\n EOF\n \n-test_expect_success 'later legacy specifiers are given precedence' '\n-\tgit config --bool --int core.number >actual &&\n-\techo 10 >expect &&\n+test_expect_success 'identical modern --type specifiers are allowed' '\n+\tgit config --type=int --type=int core.big >actual &&\n+\techo 1048576 >expect &&\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'identical legacy --type specifiers are allowed' '\n+\tgit config --int --int core.big >actual &&\n+\techo 1048576 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'identical mixed --type specifiers are allowed' '\n+\tgit config --int --type=int core.big >actual &&\n+\techo 1048576 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'non-identical modern --type specifiers are not allowed' '\n+\ttest_must_fail git config --type=int --type=bool core.big 2>error &&\n+\ttest_i18ngrep \"only one type at a time\" error\n+'\n+\n+test_expect_success 'non-identical legacy --type specifiers are not allowed' '\n+\ttest_must_fail git config --int --bool core.big 2>error &&\n+\ttest_i18ngrep \"only one type at a time\" error\n+'\n+\n+test_expect_success 'non-identical mixed --type specifiers are not allowed' '\n+\ttest_must_fail git config --type=int --bool core.big 2>error &&\n+\ttest_i18ngrep \"only one type at a time\" error\n+'\n+\n+test_expect_success '--type allows valid type specifiers' '\n+\techo \"true\" >expect &&\n+\tgit config --type=bool core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-type unsets type specifiers' '\n+\techo \"10\" >expect &&\n+\tgit config --type=bool --no-type core.number >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'unset type specifiers may be reset to conflicting ones' '\n+\techo 1048576 >expect &&\n+\tgit config --type=bool --no-type --type=int core.big >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--type rejects unknown specifiers' '\n+\ttest_must_fail git config --type=nonsense core.foo 2>error &&\n+\ttest_i18ngrep \"unrecognized --type argument\" error\n+'\n+\n test_done\n-- \n2.17.0\n\n"},{"id":"345860","messageId":"273278d4a8d741c27b31c583faed3b345ccbc2d4.1524716420.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524716420.git.me@ttaylorr.com","subject":"[PATCH 3/5] builtin/config: introduce `--default`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T04:25:25Z","receivedAt":"2018-04-26T04:25:34Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"For some use cases, callers of the `git-config(1)` builtin would like to\nfallback to default values when the variable asked for does not exist.\nIn addition, users would like to use existing type specifiers to ensure\nthat values are parsed correctly when they do exist in the\nconfiguration.\n\nFor example, to fetch a value without a type specifier and fallback to\n`$fallback`, the following is required:\n\n $ git config core.foo || echo \"$fallback\"\n\nThis is fine for most values, but can be tricky for difficult-to-express\n`$fallback`'s, like ANSI color codes.\n\nThis motivates `--get-color`, which is a one-off exception to the normal\ntype specifier rules wherein a user specifies both the configuration\nvariable and an optional fallback. Both are formatted according to their\ntype specifier, which eases the burden on the user to ensure that values\nare correctly formatted.\n\nThis commit (and those following it in this series) aim to eventually\nreplace `--get-color` with a consistent alternative. By introducing\n`--default`, we allow the `--get-color` action to be promoted to a\n`--type=color` type specifier, retaining the \"fallback\" behavior via the\n`--default` flag introduced in this commit.\n\nFor example, we aim to replace:\n\n $ git config --get-color variable [default] [...]\n\nwith:\n\n $ git config --default default --type=color variable [...]\n\nValues filled by `--default` behave exactly as if they were present in\nthe affected configuration file; they will be parsed by type specifiers\nwithout the knowledge that they are not themselves present in the\nconfiguration.\n\nSpecifically, this means that the following will work:\n\n $ git config --int --default 1M does.not.exist\n 1048576\n\nIn subsequent commits, we will offer `--type=color`, which (in\nconjunction with `--default`) will be sufficient to replace\n`--get-color`.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-config.txt | 4 ++++\n builtin/config.c | 18 ++++++++++++++++++\n t/t1310-config-default.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 58 insertions(+)\n create mode 100755 t/t1310-config-default.sh\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex b759009c75..c3adafd78a 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -240,6 +240,10 @@ Valid ``'s include:\n \tusing `--file`, `--global`, etc) and `on` when searching all\n \tconfig files.\n \n+--default ::\n+ When using `--get`, and the requested variable is not found, behave as if\n+ were the value assigned to the that variable.\n+\n CONFIGURATION\n -------------\n `pager.config` is only respected when listing configuration, i.e., when\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 2f91ef15a4..d7acf912cd 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -26,6 +26,7 @@ static char term = '\\n';\n static int use_global_config, use_system_config, use_local_config;\n static struct git_config_source given_config_source;\n static int actions, type;\n+static char *default_value;\n static int end_null;\n static int respect_includes_opt = -1;\n static struct config_options config_options;\n@@ -150,6 +151,7 @@ static struct option builtin_config_options[] = {\n \tOPT_BOOL(0, \"name-only\", &omit_values, N_(\"show variable names only\")),\n \tOPT_BOOL(0, \"includes\", &respect_includes_opt, N_(\"respect include directives on lookup\")),\n \tOPT_BOOL(0, \"show-origin\", &show_origin, N_(\"show origin of config (file, standard input, blob, command line)\")),\n+\tOPT_STRING(0, \"default\", &default_value, N_(\"value\"), N_(\"with --get, use default value when missing entry\")),\n \tOPT_END(),\n };\n \n@@ -314,6 +316,16 @@ static int get_value(const char *key_, const char *regex_)\n \tconfig_with_options(collect_config, &values,\n \t\t\t &given_config_source, &config_options);\n \n+\tif (!values.nr && default_value) {\n+\t\tstruct strbuf *item;\n+\t\tALLOC_GROW(values.items, values.nr + 1, values.alloc);\n+\t\titem = &values.items[values.nr++];\n+\t\tstrbuf_init(item, 0);\n+\t\tif (format_config(item, key_, default_value) < 0)\n+\t\t\tdie(_(\"failed to format default config value: %s\"),\n+\t\t\t\tdefault_value);\n+\t}\n+\n \tret = !values.nr;\n \n \tfor (i = 0; i < values.nr; i++) {\n@@ -652,6 +664,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n \n+\tif (default_value && !(actions & ACTION_GET)) {\n+\t\terror(\"--default is only applicable to --get\");\n+\t\tusage_with_options(builtin_config_usage,\n+\t\t\tbuiltin_config_options);\n+\t}\n+\n \tif (actions & PAGING_ACTIONS)\n \t\tsetup_auto_pager(\"config\", 1);\n \ndiff --git a/t/t1310-config-default.sh b/t/t1310-config-default.sh\nnew file mode 100755\nindex 0000000000..6049d91708\n--- /dev/null\n+++ b/t/t1310-config-default.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+test_description='Test git config in different settings (with --default)'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'uses --default when entry missing' '\n+\techo quux >expect &&\n+\tgit config -f config --default=quux core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'does not use --default when entry present' '\n+\techo bar >expect &&\n+\tgit -c core.foo=bar config --default=baz core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'canonicalizes --default with appropriate type' '\n+\techo true >expect &&\n+\tgit config -f config --default=yes --bool core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'dies when --default cannot be parsed' '\n+\ttest_must_fail git config -f config --type=expiry-date --default=x --get \\\n+\t\tnot.a.section 2>error &&\n+\ttest_i18ngrep \"failed to format default config value\" error\n+'\n+\n+test_expect_success 'does not allow --default without --get' '\n+\ttest_must_fail git config --default=quux --unset a.section >output 2>&1 &&\n+\ttest_i18ngrep \"\\-\\-default is only applicable to\" output\n+'\n+\n+test_done\n-- \n2.17.0\n\n"},{"id":"345862","messageId":"9af7952db38403dc7ed7aed1fea66d12c3cd5361.1524716420.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524716420.git.me@ttaylorr.com","subject":"[PATCH 4/5] config.c: introduce 'git_config_color' to parse ANSI colors","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T04:25:27Z","receivedAt":"2018-04-26T04:25:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding `--type=color` to the `git-config(1)` builtin,\nlet's introduce a color parsing utility, `git_config_color` in a similar\nfashion to `git_config_`.\n\nSigned-off-by: Taylor Blau \n---\n config.c | 10 ++++++++++\n config.h | 1 +\n 2 files changed, 11 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex 62c56099bf..d14813bef7 100644\n--- a/config.c\n+++ b/config.c\n@@ -16,6 +16,7 @@\n #include \"string-list.h\"\n #include \"utf8.h\"\n #include \"dir.h\"\n+#include \"color.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -1000,6 +1001,15 @@ int git_config_expiry_date(timestamp_t *timestamp, const char *var, const char *\n \treturn 0;\n }\n \n+int git_config_color(char *dest, const char *var, const char *value)\n+{\n+\tif (!value)\n+\t\treturn config_error_nonbool(var);\n+\tif (color_parse(value, dest) < 0)\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n static int git_default_core_config(const char *var, const char *value)\n {\n \t/* This needs a better name */\ndiff --git a/config.h b/config.h\nindex ef70a9cac1..0e060779d9 100644\n--- a/config.h\n+++ b/config.h\n@@ -59,6 +59,7 @@ extern int git_config_bool(const char *, const char *);\n extern int git_config_string(const char **, const char *, const char *);\n extern int git_config_pathname(const char **, const char *, const char *);\n extern int git_config_expiry_date(timestamp_t *, const char *, const char *);\n+extern int git_config_color(char *, const char *, const char *);\n extern int git_config_set_in_file_gently(const char *, const char *, const char *);\n extern void git_config_set_in_file(const char *, const char *, const char *);\n extern int git_config_set_gently(const char *, const char *);\n-- \n2.17.0\n\n"},{"id":"345863","messageId":"17188b9f3250e4d3547f81b4aaf70a209941b1b5.1524716420.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524716420.git.me@ttaylorr.com","subject":"[PATCH 5/5] builtin/config: introduce `color` type specifier","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T04:25:29Z","receivedAt":"2018-04-26T04:25:41Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"As of this commit, the canonical way to retreive an ANSI-compatible\ncolor escape sequence from a configuration file is with the\n`--get-color` action.\n\nThis is to allow Git to \"fall back\" on a default value for the color\nshould the given section not exist in the specified configuration(s).\n\nWith the addition of `--default`, this is no longer needed since:\n\n $ git config --default red --type=color core.section\n\nwill be have exactly as:\n\n $ git config --get-color core.section red\n\nFor consistency, let's introduce `--type=color` and encourage its use\nwith `--default` together over `--get-color` alone.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-config.txt | 6 ++++++\n builtin/config.c | 20 ++++++++++++++++++++\n t/t1300-repo-config.sh | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 56 insertions(+)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex c3adafd78a..18ddc78f42 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -177,6 +177,10 @@ Valid ``'s include:\n ~/` from the command line to let your shell do the expansion.)\n - 'expiry-date': canonicalize by converting from a fixed or relative date-string\n to a timestamp. This specifier has no effect when setting the value.\n+- 'color': When getting a value, canonicalize by converting to an ANSI color\n+ escape sequence. When setting a value, a sanity-check is performed to ensure\n+ that the given value is canonicalize-able as an ANSI color, but it is written\n+ as-is.\n +\n \n --bool::\n@@ -228,6 +232,8 @@ Valid ``'s include:\n \toutput it as the ANSI color escape sequence to the standard\n \toutput. The optional `default` parameter is used instead, if\n \tthere is no color configured for `name`.\n++\n+`--type=color [--default=]` is preferred over `--get-color`.\n \n -e::\n --edit::\ndiff --git a/builtin/config.c b/builtin/config.c\nindex d7acf912cd..ec5c11293b 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -61,6 +61,7 @@ static int show_origin;\n #define TYPE_BOOL_OR_INT\t3\n #define TYPE_PATH\t\t4\n #define TYPE_EXPIRY_DATE\t5\n+#define TYPE_COLOR\t\t6\n \n #define OPT_CALLBACK_VALUE(s, l, v, h, i) \\\n \t{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \\\n@@ -231,6 +232,11 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value\n \t\t\tif (git_config_expiry_date(&t, key_, value_) < 0)\n \t\t\t\treturn -1;\n \t\t\tstrbuf_addf(buf, \"%\"PRItime, t);\n+\t\t} else if (type == TYPE_COLOR) {\n+\t\t\tchar v[COLOR_MAXLEN];\n+\t\t\tif (git_config_color(v, key_, value_) < 0)\n+\t\t\t\treturn -1;\n+\t\t\tstrbuf_addstr(buf, v);\n \t\t} else if (value_) {\n \t\t\tstrbuf_addstr(buf, value_);\n \t\t} else {\n@@ -376,6 +382,20 @@ static char *normalize_value(const char *key, const char *value)\n \t\telse\n \t\t\treturn xstrdup(v ? \"true\" : \"false\");\n \t}\n+\tif (type == TYPE_COLOR) {\n+\t\tchar v[COLOR_MAXLEN];\n+\t\tif (git_config_color(v, key, value))\n+\t\t\tdie(\"cannot parse color '%s'\", value);\n+\n+\t\t/*\n+\t\t * The contents of `v` now contain an ANSI escape\n+\t\t * sequence, not suitable for including within a\n+\t\t * configuration file. Treat the above as a\n+\t\t * \"sanity-check\", and return the given value, which we\n+\t\t * know is representable as valid color code.\n+\t\t */\n+\t\treturn xstrdup(value);\n+\t}\n \n \tdie(\"BUG: cannot normalize type %d\", type);\n }\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 1e3a1ace14..2acfd513f1 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -931,6 +931,36 @@ test_expect_success 'get --expiry-date' '\n \ttest_must_fail git config --expiry-date date.invalid1\n '\n \n+test_expect_success 'get --type=color' '\n+\trm .git/config &&\n+\tgit config foo.color \"red\" &&\n+\tgit config --get --type=color foo.color >actual.raw &&\n+\ttest_decode_color actual &&\n+\techo \"\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect << EOF\n+[foo]\n+\tcolor = red\n+EOF\n+\n+test_expect_success 'set --type=color' '\n+\trm .git/config &&\n+\tgit config --type=color foo.color \"red\" &&\n+\ttest_cmp expect .git/config\n+'\n+\n+test_expect_success 'get --type=color barfs on non-color' '\n+\techo \"[foo]bar=not-a-color\" >.git/config &&\n+\ttest_must_fail git config --get --type=color foo.bar\n+'\n+\n+test_expect_success 'set --type=color barfs on non-color' '\n+\ttest_must_fail git config --type=color foo.color \"not-a-color\" 2>error &&\n+\ttest_i18ngrep \"cannot parse color\" error\n+'\n+\n cat > expect << EOF\n [quote]\n \tleading = \" test\"\n-- \n2.17.0\n"},{"id":"345865","messageId":"xmqq7eou35ev.fsf@gitster-ct.c.googlers.com","threadId":"48361","inReplyTo":"1bc42a611089a2552b1d8bc83ff229be0a5136ff.1524716420.git.me@ttaylorr.com","subject":"Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-26T05:25:44Z","receivedAt":"2018-04-26T05:25:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`\n\nI'd retitle while queuing, as the last 'type' is a placeholder for\nconcrete types like above.\n\n> +static int option_parse_type(const struct option *opt, const char *arg,\n> +\t\t\t int unset)\n> +{\n> +\tint new_type;\n> +\tint *to_type;\n\nSplitting these into two lines (unlike placing on a single same\nline, which was how the previous round was queued) like this is\ngood.\n\n> +...\n> +\tnew_type = opt->defval;\n> +\tif (!new_type) {\n> +...\n> +\t}\n> +\n> +\t*to_type = opt->value;\n\nBut this is wrong, no? You meant opt->value points at an integer\nvariable that receives the type we discover by parsing, i.e.\n\n\tto_type = opt->value;\n\n"},{"id":"345866","messageId":"xmqq36zi352x.fsf@gitster-ct.c.googlers.com","threadId":"48361","inReplyTo":"17188b9f3250e4d3547f81b4aaf70a209941b1b5.1524716420.git.me@ttaylorr.com","subject":"Re: [PATCH 5/5] builtin/config: introduce `color` type specifier","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-26T05:32:54Z","receivedAt":"2018-04-26T05:33:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> diff --git a/builtin/config.c b/builtin/config.c\n> index d7acf912cd..ec5c11293b 100644\n> --- a/builtin/config.c\n> +++ b/builtin/config.c\n> @@ -61,6 +61,7 @@ static int show_origin;\n> #define TYPE_BOOL_OR_INT\t3\n> #define TYPE_PATH\t\t4\n> #define TYPE_EXPIRY_DATE\t5\n> +#define TYPE_COLOR\t\t6\n> \n> #define OPT_CALLBACK_VALUE(s, l, v, h, i) \\\n> \t{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \\\n> @@ -231,6 +232,11 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value\n> \t\t\tif (git_config_expiry_date(&t, key_, value_) < 0)\n> \t\t\t\treturn -1;\n> \t\t\tstrbuf_addf(buf, \"%\"PRItime, t);\n> +\t\t} else if (type == TYPE_COLOR) {\n> +\t\t\tchar v[COLOR_MAXLEN];\n> +\t\t\tif (git_config_color(v, key_, value_) < 0)\n> +\t\t\t\treturn -1;\n> +\t\t\tstrbuf_addstr(buf, v);\n> \t\t} else if (value_) {\n> \t\t\tstrbuf_addstr(buf, value_);\n> \t\t} else {\n> @@ -376,6 +382,20 @@ static char *normalize_value(const char *key, const char *value)\n> \t\telse\n> \t\t\treturn xstrdup(v ? \"true\" : \"false\");\n> \t}\n> +\tif (type == TYPE_COLOR) {\n> +\t\tchar v[COLOR_MAXLEN];\n> +\t\tif (git_config_color(v, key, value))\n> +\t\t\tdie(\"cannot parse color '%s'\", value);\n> +\n> +\t\t/*\n> +\t\t * The contents of `v` now contain an ANSI escape\n> +\t\t * sequence, not suitable for including within a\n> +\t\t * configuration file. Treat the above as a\n> +\t\t * \"sanity-check\", and return the given value, which we\n> +\t\t * know is representable as valid color code.\n> +\t\t */\n> +\t\treturn xstrdup(value);\n> +\t}\n> \n> \tdie(\"BUG: cannot normalize type %d\", type);\n> }\n\nHmmm, option_parse_type() introduced in [PATCH 2/5] used to learn\nto parse \"color\" in this step, but it no longer does. \n\nHere is the difference between what has been queued and the result\nof applying these latest patches I found.\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 69e7270356..ec5c11293b 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -72,7 +72,8 @@ static struct option builtin_config_options[];\n static int option_parse_type(const struct option *opt, const char *arg,\n \t\t\t int unset)\n {\n-\tint new_type, *to_type;\n+\tint new_type;\n+\tint *to_type;\n \n \tif (unset) {\n \t\t*((int *) opt->value) = 0;\n@@ -95,13 +96,11 @@ static int option_parse_type(const struct option *opt, const char *arg,\n \t\t\tnew_type = TYPE_PATH;\n \t\telse if (!strcmp(arg, \"expiry-date\"))\n \t\t\tnew_type = TYPE_EXPIRY_DATE;\n-\t\telse if (!strcmp(arg, \"color\"))\n-\t\t\tnew_type = TYPE_COLOR;\n \t\telse\n \t\t\tdie(_(\"unrecognized --type argument, %s\"), arg);\n \t}\n \n-\tto_type = opt->value;\n+\t*to_type = opt->value;\n \tif (*to_type && *to_type != new_type) {\n \t\t/*\n \t\t * Complain when there is a new type not equal to the old type.\n"},{"id":"345867","messageId":"cover.1524722247.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524716420.git.me@ttaylorr.com","subject":"[PATCH v2 0/5] builtin/config.c: combined series '--type', '--default'","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T05:58:25Z","receivedAt":"2018-04-26T05:58:32Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Hi,\n\nHere is an amended version of my combined series to add '--type' and\n'--default'. My apologies for the re-roll, I thought that I had looked\neverything over closely enough :-).\n\nSince last time:\n\n * Correct an obviously-wrong assignment into '*to_type' [1]. I have\n moved both of these assignments into the top-line declaration of\n those variables.\n\n * Re-add a removed hunk to support '--type=color' correctly [2].\n\nThanks,\nTaylor\n\n[1]: https://public-inbox.org/git/xmqq7eou35ev.fsf@gitster-ct.c.googlers.com\n[2]: https://public-inbox.org/git/xmqq36zi352x.fsf@gitster-ct.c.googlers.com\n\nTaylor Blau (5):\n builtin/config.c: treat type specifiers singularly\n builtin/config.c: support `--type=` as preferred alias for\n `--`\n builtin/config: introduce `--default`\n config.c: introduce 'git_config_color' to parse ANSI colors\n builtin/config: introduce `color` type specifier\n\n Documentation/git-config.txt | 81 ++++++++++++--------\n builtin/config.c | 143 ++++++++++++++++++++++++++++-------\n config.c | 10 +++\n config.h | 1 +\n t/t1300-repo-config.sh | 93 +++++++++++++++++++++++\n t/t1310-config-default.sh | 36 +++++++++\n 6 files changed, 305 insertions(+), 59 deletions(-)\n create mode 100755 t/t1310-config-default.sh\n\nInter-diff (since v1):\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex ec5c11293b..bb62816bba 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -72,19 +72,18 @@ static struct option builtin_config_options[];\n static int option_parse_type(const struct option *opt, const char *arg,\n \t\t\t int unset)\n {\n-\tint new_type;\n-\tint *to_type;\n+\t/*\n+\t * To support '--' style flags, begin with new_type equal to\n+\t * opt->defval.\n+\t */\n+\tint new_type = opt->defval;\n+\tint *to_type = opt->value;\n\n \tif (unset) {\n \t\t*((int *) opt->value) = 0;\n \t\treturn 0;\n \t}\n\n-\t/*\n-\t * To support '--' style flags, begin with new_type equal to\n-\t * opt->defval.\n-\t */\n-\tnew_type = opt->defval;\n \tif (!new_type) {\n \t\tif (!strcmp(arg, \"bool\"))\n \t\t\tnew_type = TYPE_BOOL;\n@@ -96,11 +95,12 @@ static int option_parse_type(const struct option *opt, const char *arg,\n \t\t\tnew_type = TYPE_PATH;\n \t\telse if (!strcmp(arg, \"expiry-date\"))\n \t\t\tnew_type = TYPE_EXPIRY_DATE;\n+\t\telse if (!strcmp(arg, \"color\"))\n+\t\t\tnew_type = TYPE_COLOR;\n \t\telse\n \t\t\tdie(_(\"unrecognized --type argument, %s\"), arg);\n \t}\n\n-\t*to_type = opt->value;\n \tif (*to_type && *to_type != new_type) {\n \t\t/*\n \t\t * Complain when there is a new type not equal to the old type.\n\n--\n2.17.0\n"},{"id":"345868","messageId":"2a0e25c3906622e49bcbd772eb8872442c03fc37.1524722247.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524722247.git.me@ttaylorr.com","subject":"[PATCH v2 1/5] builtin/config.c: treat type specifiers singularly","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T05:58:28Z","receivedAt":"2018-04-26T05:58:36Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"Internally, we represent `git config`'s type specifiers as a bitset\nusing OPT_BIT. 'bool' is 1<<0, 'int' is 1<<1, and so on. This technique\nallows for the representation of multiple type specifiers in the `int\ntypes` field, but this multi-representation is left unused.\n\nIn fact, `git config` will not accept multiple type specifiers at a\ntime, as indicated by:\n\n $ git config --int --bool some.section\n error: only one type at a time.\n\nThis patch uses `OPT_SET_INT` to prefer the _last_ mentioned type\nspecifier, so that the above command would instead be valid, and a\nsynonym of:\n\n $ git config --bool some.section\n\nThis change is motivated by two urges: (1) it does not make sense to\nrepresent a singular type specifier internally as a bitset, only to\ncomplain when there are multiple bits in the set. `OPT_SET_INT` is more\nwell-suited to this task than `OPT_BIT` is. (2) a future patch will\nintroduce `--type=`, and we would like not to complain in the\nfollowing situation:\n\n $ git config --int --type=int\n\nSigned-off-by: Taylor Blau \n---\n builtin/config.c | 49 +++++++++++++++++++-----------------------\n t/t1300-repo-config.sh | 11 ++++++++++\n 2 files changed, 33 insertions(+), 27 deletions(-)\n\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 01169dd628..92fb8d56b1 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -25,7 +25,7 @@ static char term = '\\n';\n \n static int use_global_config, use_system_config, use_local_config;\n static struct git_config_source given_config_source;\n-static int actions, types;\n+static int actions, type;\n static int end_null;\n static int respect_includes_opt = -1;\n static struct config_options config_options;\n@@ -55,11 +55,11 @@ static int show_origin;\n #define PAGING_ACTIONS (ACTION_LIST | ACTION_GET_ALL | \\\n \t\t\tACTION_GET_REGEXP | ACTION_GET_URLMATCH)\n \n-#define TYPE_BOOL (1<<0)\n-#define TYPE_INT (1<<1)\n-#define TYPE_BOOL_OR_INT (1<<2)\n-#define TYPE_PATH (1<<3)\n-#define TYPE_EXPIRY_DATE (1<<4)\n+#define TYPE_BOOL\t\t1\n+#define TYPE_INT\t\t2\n+#define TYPE_BOOL_OR_INT\t3\n+#define TYPE_PATH\t\t4\n+#define TYPE_EXPIRY_DATE\t5\n \n static struct option builtin_config_options[] = {\n \tOPT_GROUP(N_(\"Config file location\")),\n@@ -84,11 +84,11 @@ static struct option builtin_config_options[] = {\n \tOPT_BIT(0, \"get-color\", &actions, N_(\"find the color configured: slot [default]\"), ACTION_GET_COLOR),\n \tOPT_BIT(0, \"get-colorbool\", &actions, N_(\"find the color setting: slot [stdout-is-tty]\"), ACTION_GET_COLORBOOL),\n \tOPT_GROUP(N_(\"Type\")),\n-\tOPT_BIT(0, \"bool\", &types, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n-\tOPT_BIT(0, \"int\", &types, N_(\"value is decimal number\"), TYPE_INT),\n-\tOPT_BIT(0, \"bool-or-int\", &types, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n-\tOPT_BIT(0, \"path\", &types, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n-\tOPT_BIT(0, \"expiry-date\", &types, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n+\tOPT_SET_INT(0, \"bool\", &type, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n+\tOPT_SET_INT(0, \"int\", &type, N_(\"value is decimal number\"), TYPE_INT),\n+\tOPT_SET_INT(0, \"bool-or-int\", &type, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n+\tOPT_SET_INT(0, \"path\", &type, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n+\tOPT_SET_INT(0, \"expiry-date\", &type, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n \tOPT_GROUP(N_(\"Other\")),\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@@ -149,26 +149,26 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value\n \t\tif (show_keys)\n \t\t\tstrbuf_addch(buf, key_delim);\n \n-\t\tif (types == TYPE_INT)\n+\t\tif (type == TYPE_INT)\n \t\t\tstrbuf_addf(buf, \"%\"PRId64,\n \t\t\t\t git_config_int64(key_, value_ ? value_ : \"\"));\n-\t\telse if (types == TYPE_BOOL)\n+\t\telse if (type == TYPE_BOOL)\n \t\t\tstrbuf_addstr(buf, git_config_bool(key_, value_) ?\n \t\t\t\t \"true\" : \"false\");\n-\t\telse if (types == TYPE_BOOL_OR_INT) {\n+\t\telse if (type == TYPE_BOOL_OR_INT) {\n \t\t\tint is_bool, v;\n \t\t\tv = git_config_bool_or_int(key_, value_, &is_bool);\n \t\t\tif (is_bool)\n \t\t\t\tstrbuf_addstr(buf, v ? \"true\" : \"false\");\n \t\t\telse\n \t\t\t\tstrbuf_addf(buf, \"%d\", v);\n-\t\t} else if (types == TYPE_PATH) {\n+\t\t} else if (type == TYPE_PATH) {\n \t\t\tconst char *v;\n \t\t\tif (git_config_pathname(&v, key_, value_) < 0)\n \t\t\t\treturn -1;\n \t\t\tstrbuf_addstr(buf, v);\n \t\t\tfree((char *)v);\n-\t\t} else if (types == TYPE_EXPIRY_DATE) {\n+\t\t} else if (type == TYPE_EXPIRY_DATE) {\n \t\t\ttimestamp_t t;\n \t\t\tif (git_config_expiry_date(&t, key_, value_) < 0)\n \t\t\t\treturn -1;\n@@ -287,7 +287,7 @@ static char *normalize_value(const char *key, const char *value)\n \tif (!value)\n \t\treturn NULL;\n \n-\tif (types == 0 || types == TYPE_PATH || types == TYPE_EXPIRY_DATE)\n+\tif (type == 0 || type == TYPE_PATH || type == TYPE_EXPIRY_DATE)\n \t\t/*\n \t\t * We don't do normalization for TYPE_PATH here: If\n \t\t * the path is like ~/foobar/, we prefer to store\n@@ -296,11 +296,11 @@ static char *normalize_value(const char *key, const char *value)\n \t\t * Also don't do normalization for expiry dates.\n \t\t */\n \t\treturn xstrdup(value);\n-\tif (types == TYPE_INT)\n+\tif (type == TYPE_INT)\n \t\treturn xstrfmt(\"%\"PRId64, git_config_int64(key, value));\n-\tif (types == TYPE_BOOL)\n+\tif (type == TYPE_BOOL)\n \t\treturn xstrdup(git_config_bool(key, value) ? \"true\" : \"false\");\n-\tif (types == TYPE_BOOL_OR_INT) {\n+\tif (type == TYPE_BOOL_OR_INT) {\n \t\tint is_bool, v;\n \t\tv = git_config_bool_or_int(key, value, &is_bool);\n \t\tif (!is_bool)\n@@ -309,7 +309,7 @@ static char *normalize_value(const char *key, const char *value)\n \t\t\treturn xstrdup(v ? \"true\" : \"false\");\n \t}\n \n-\tdie(\"BUG: cannot normalize type %d\", types);\n+\tdie(\"BUG: cannot normalize type %d\", type);\n }\n \n static int get_color_found;\n@@ -566,12 +566,7 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tkey_delim = '\\n';\n \t}\n \n-\tif (HAS_MULTI_BITS(types)) {\n-\t\terror(\"only one type at a time.\");\n-\t\tusage_with_options(builtin_config_usage, builtin_config_options);\n-\t}\n-\n-\tif ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && types) {\n+\tif ((actions & (ACTION_GET_COLOR|ACTION_GET_COLORBOOL)) && type) {\n \t\terror(\"--get-color and variable type are incoherent\");\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex e95b1e67da..ed9b9cd344 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1611,4 +1611,15 @@ test_expect_success '--local requires a repo' '\n \ttest_expect_code 128 nongit git config --local foo.bar\n '\n \n+cat >.git/config <<-\\EOF &&\n+[core]\n+number = 10\n+EOF\n+\n+test_expect_success 'later legacy specifiers are given precedence' '\n+\tgit config --bool --int core.number >actual &&\n+\techo 10 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.17.0\n\n"},{"id":"345869","messageId":"404da8bdeb9808d2dfcfdf09bc62c76195c9a08c.1524722247.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524722247.git.me@ttaylorr.com","subject":"[PATCH v2 2/5] builtin/config.c: support `--type=` as preferred alias for `--`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T05:58:32Z","receivedAt":"2018-04-26T05:58:40Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"`git config` has long allowed the ability for callers to provide a 'type\nspecifier', which instructs `git config` to (1) ensure that incoming\nvalues can be interpreted as that type, and (2) that outgoing values are\ncanonicalized under that type.\n\nIn another series, we propose to extend this functionality with\n`--type=color` and `--default` to replace `--get-color`.\n\nHowever, we traditionally use `--color` to mean \"colorize this output\",\ninstead of \"this value should be treated as a color\".\n\nCurrently, `git config` does not support this kind of colorization, but\nwe should be careful to avoid squatting on this option too soon, so that\n`git config` can support `--color` (in the traditional sense) in the\nfuture, if that is desired.\n\nIn this patch, we support `--type=` in\naddition to `--int`, `--bool`, and etc. This allows the aforementioned\nupcoming patch to support querying a color value with a default via\n`--type=color --default=...`, without squandering `--color`.\n\nWe retain the historic behavior of complaining when multiple,\nlegacy-style `--` flags are given, as well as extend this to\nconflicting new-style `--type=` flags. `--int --type=int` (and its\ncommutative pair) does not complain, but `--bool --type=int` (and its\ncommutative pair) does.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-config.txt | 71 ++++++++++++++++++++----------------\n builtin/config.c | 64 +++++++++++++++++++++++++++++---\n t/t1300-repo-config.sh | 58 +++++++++++++++++++++++++++--\n 3 files changed, 153 insertions(+), 40 deletions(-)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex e09ed5d7d5..b759009c75 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -9,13 +9,13 @@ git-config - Get and set repository or global options\n SYNOPSIS\n --------\n [verse]\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] [--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' [] [--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=] [--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@@ -38,12 +38,10 @@ existing values that match the regexp are updated or unset. If\n you want to handle the lines that do *not* match the regex, just\n prepend a single exclamation mark in front (see also <>).\n \n-The type specifier can be either `--int` or `--bool`, to make\n-'git config' ensure that the variable(s) are of the given type and\n-convert the value to the canonical form (simple decimal number for int,\n-a \"true\" or \"false\" string for bool), or `--path`, which does some\n-path expansion (see `--path` below). If no type specifier is passed, no\n-checks or transformations are performed on the value.\n+The `--type=` option instructs 'git config' to ensure that incoming and\n+outgoing values are canonicalize-able under the given . If no\n+`--type=` is given, no canonicalization will be performed. Callers may\n+unset an existing `--type` specifier with `--no-type`.\n \n When reading, the values are read from the system, global and\n repository local configuration files by default, and options\n@@ -160,30 +158,39 @@ See also <>.\n --list::\n \tList all variables set in config file, along with their values.\n \n---bool::\n-\t'git config' will ensure that the output is \"true\" or \"false\"\n+--type ::\n+ 'git config' will ensure that any input or output is valid under the given\n+ type constraint(s), and will canonicalize outgoing values in ``'s\n+ canonical form.\n++\n+Valid ``'s include:\n++\n+- 'bool': canonicalize values as either \"true\" or \"false\".\n+- 'int': canonicalize values as simple decimal numbers. An optional suffix of\n+ 'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or\n+ 1073741824 upon input.\n+- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described\n+ above.\n+- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and\n+ `~user` to the home directory for the specified user. This specifier has no\n+ effect when setting the value (but you can use `git config section.variable\n+ ~/` from the command line to let your shell do the expansion.)\n+- 'expiry-date': canonicalize by converting from a fixed or relative date-string\n+ to a timestamp. This specifier has no effect when setting the value.\n++\n \n+--bool::\n --int::\n-\t'git config' will ensure that the output is a simple\n-\tdecimal number. An optional value suffix of 'k', 'm', or 'g'\n-\tin the config file will cause the value to be multiplied\n-\tby 1024, 1048576, or 1073741824 prior to output.\n-\n --bool-or-int::\n-\t'git config' will ensure that the output matches the format of\n-\teither --bool or --int, as described above.\n-\n --path::\n-\t`git config` will expand a leading `~` to the value of\n-\t`$HOME`, and `~user` to the home directory for the\n-\tspecified user. This option has no effect when setting the\n-\tvalue (but you can use `git config section.variable ~/`\n-\tfrom the command line to let your shell do the expansion).\n-\n --expiry-date::\n-\t`git config` will ensure that the output is converted from\n-\ta fixed or relative date-string to a timestamp. This option\n-\thas no effect when setting the value.\n+ Historical options for selecting a type specifier. Prefer instead `--type`,\n+ (see: above).\n+\n+--no-type::\n+ Un-sets the previously set type specifier (if one was previously set). This\n+ option requests that 'git config' not canonicalize the retrieved variable.\n+ `--no-type` has no effect without `--type=` or `--`.\n \n -z::\n --null::\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 92fb8d56b1..9eda01615b 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -61,6 +61,59 @@ static int show_origin;\n #define TYPE_PATH\t\t4\n #define TYPE_EXPIRY_DATE\t5\n \n+#define OPT_CALLBACK_VALUE(s, l, v, h, i) \\\n+\t{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \\\n+\tPARSE_OPT_NONEG, option_parse_type, (i) }\n+\n+static struct option builtin_config_options[];\n+\n+static int option_parse_type(const struct option *opt, const char *arg,\n+\t\t\t int unset)\n+{\n+\t/*\n+\t * To support '--' style flags, begin with new_type equal to\n+\t * opt->defval.\n+\t */\n+\tint new_type = opt->defval;\n+\tint *to_type = opt->value;\n+\n+\tif (unset) {\n+\t\t*((int *) opt->value) = 0;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!new_type) {\n+\t\tif (!strcmp(arg, \"bool\"))\n+\t\t\tnew_type = TYPE_BOOL;\n+\t\telse if (!strcmp(arg, \"int\"))\n+\t\t\tnew_type = TYPE_INT;\n+\t\telse if (!strcmp(arg, \"bool-or-int\"))\n+\t\t\tnew_type = TYPE_BOOL_OR_INT;\n+\t\telse if (!strcmp(arg, \"path\"))\n+\t\t\tnew_type = TYPE_PATH;\n+\t\telse if (!strcmp(arg, \"expiry-date\"))\n+\t\t\tnew_type = TYPE_EXPIRY_DATE;\n+\t\telse\n+\t\t\tdie(_(\"unrecognized --type argument, %s\"), arg);\n+\t}\n+\n+\tif (*to_type && *to_type != new_type) {\n+\t\t/*\n+\t\t * Complain when there is a new type not equal to the old type.\n+\t\t * This allows for combinations like '--int --type=int' and\n+\t\t * '--type=int --type=int', but disallows ones like '--type=bool\n+\t\t * --int' and '--type=bool\n+\t\t * --type=int'.\n+\t\t */\n+\t\terror(\"only one type at a time.\");\n+\t\tusage_with_options(builtin_config_usage,\n+\t\t\tbuiltin_config_options);\n+\t}\n+\t*to_type = new_type;\n+\n+\treturn 0;\n+}\n+\n static struct option builtin_config_options[] = {\n \tOPT_GROUP(N_(\"Config file location\")),\n \tOPT_BOOL(0, \"global\", &use_global_config, N_(\"use global config file\")),\n@@ -84,11 +137,12 @@ static struct option builtin_config_options[] = {\n \tOPT_BIT(0, \"get-color\", &actions, N_(\"find the color configured: slot [default]\"), ACTION_GET_COLOR),\n \tOPT_BIT(0, \"get-colorbool\", &actions, N_(\"find the color setting: slot [stdout-is-tty]\"), ACTION_GET_COLORBOOL),\n \tOPT_GROUP(N_(\"Type\")),\n-\tOPT_SET_INT(0, \"bool\", &type, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n-\tOPT_SET_INT(0, \"int\", &type, N_(\"value is decimal number\"), TYPE_INT),\n-\tOPT_SET_INT(0, \"bool-or-int\", &type, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n-\tOPT_SET_INT(0, \"path\", &type, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n-\tOPT_SET_INT(0, \"expiry-date\", &type, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n+\tOPT_CALLBACK('t', \"type\", &type, \"\", N_(\"value is given this type\"), option_parse_type),\n+\tOPT_CALLBACK_VALUE(0, \"bool\", &type, N_(\"value is \\\"true\\\" or \\\"false\\\"\"), TYPE_BOOL),\n+\tOPT_CALLBACK_VALUE(0, \"int\", &type, N_(\"value is decimal number\"), TYPE_INT),\n+\tOPT_CALLBACK_VALUE(0, \"bool-or-int\", &type, N_(\"value is --bool or --int\"), TYPE_BOOL_OR_INT),\n+\tOPT_CALLBACK_VALUE(0, \"path\", &type, N_(\"value is a path (file or directory name)\"), TYPE_PATH),\n+\tOPT_CALLBACK_VALUE(0, \"expiry-date\", &type, N_(\"value is an expiry date\"), TYPE_EXPIRY_DATE),\n \tOPT_GROUP(N_(\"Other\")),\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\")),\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex ed9b9cd344..1e3a1ace14 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -1613,13 +1613,65 @@ test_expect_success '--local requires a repo' '\n \n cat >.git/config <<-\\EOF &&\n [core]\n+foo = true\n number = 10\n+big = 1M\n EOF\n \n-test_expect_success 'later legacy specifiers are given precedence' '\n-\tgit config --bool --int core.number >actual &&\n-\techo 10 >expect &&\n+test_expect_success 'identical modern --type specifiers are allowed' '\n+\tgit config --type=int --type=int core.big >actual &&\n+\techo 1048576 >expect &&\n \ttest_cmp expect actual\n '\n \n+test_expect_success 'identical legacy --type specifiers are allowed' '\n+\tgit config --int --int core.big >actual &&\n+\techo 1048576 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'identical mixed --type specifiers are allowed' '\n+\tgit config --int --type=int core.big >actual &&\n+\techo 1048576 >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'non-identical modern --type specifiers are not allowed' '\n+\ttest_must_fail git config --type=int --type=bool core.big 2>error &&\n+\ttest_i18ngrep \"only one type at a time\" error\n+'\n+\n+test_expect_success 'non-identical legacy --type specifiers are not allowed' '\n+\ttest_must_fail git config --int --bool core.big 2>error &&\n+\ttest_i18ngrep \"only one type at a time\" error\n+'\n+\n+test_expect_success 'non-identical mixed --type specifiers are not allowed' '\n+\ttest_must_fail git config --type=int --bool core.big 2>error &&\n+\ttest_i18ngrep \"only one type at a time\" error\n+'\n+\n+test_expect_success '--type allows valid type specifiers' '\n+\techo \"true\" >expect &&\n+\tgit config --type=bool core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--no-type unsets type specifiers' '\n+\techo \"10\" >expect &&\n+\tgit config --type=bool --no-type core.number >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'unset type specifiers may be reset to conflicting ones' '\n+\techo 1048576 >expect &&\n+\tgit config --type=bool --no-type --type=int core.big >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success '--type rejects unknown specifiers' '\n+\ttest_must_fail git config --type=nonsense core.foo 2>error &&\n+\ttest_i18ngrep \"unrecognized --type argument\" error\n+'\n+\n test_done\n-- \n2.17.0\n\n"},{"id":"345870","messageId":"52b4b8962ea8668c3d89e1549465128a25c7dfcb.1524722247.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524722247.git.me@ttaylorr.com","subject":"[PATCH v2 3/5] builtin/config: introduce `--default`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T05:58:34Z","receivedAt":"2018-04-26T05:58:43Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"For some use cases, callers of the `git-config(1)` builtin would like to\nfallback to default values when the variable asked for does not exist.\nIn addition, users would like to use existing type specifiers to ensure\nthat values are parsed correctly when they do exist in the\nconfiguration.\n\nFor example, to fetch a value without a type specifier and fallback to\n`$fallback`, the following is required:\n\n $ git config core.foo || echo \"$fallback\"\n\nThis is fine for most values, but can be tricky for difficult-to-express\n`$fallback`'s, like ANSI color codes.\n\nThis motivates `--get-color`, which is a one-off exception to the normal\ntype specifier rules wherein a user specifies both the configuration\nvariable and an optional fallback. Both are formatted according to their\ntype specifier, which eases the burden on the user to ensure that values\nare correctly formatted.\n\nThis commit (and those following it in this series) aim to eventually\nreplace `--get-color` with a consistent alternative. By introducing\n`--default`, we allow the `--get-color` action to be promoted to a\n`--type=color` type specifier, retaining the \"fallback\" behavior via the\n`--default` flag introduced in this commit.\n\nFor example, we aim to replace:\n\n $ git config --get-color variable [default] [...]\n\nwith:\n\n $ git config --default default --type=color variable [...]\n\nValues filled by `--default` behave exactly as if they were present in\nthe affected configuration file; they will be parsed by type specifiers\nwithout the knowledge that they are not themselves present in the\nconfiguration.\n\nSpecifically, this means that the following will work:\n\n $ git config --int --default 1M does.not.exist\n 1048576\n\nIn subsequent commits, we will offer `--type=color`, which (in\nconjunction with `--default`) will be sufficient to replace\n`--get-color`.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-config.txt | 4 ++++\n builtin/config.c | 18 ++++++++++++++++++\n t/t1310-config-default.sh | 36 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 58 insertions(+)\n create mode 100755 t/t1310-config-default.sh\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex b759009c75..c3adafd78a 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -240,6 +240,10 @@ Valid ``'s include:\n \tusing `--file`, `--global`, etc) and `on` when searching all\n \tconfig files.\n \n+--default ::\n+ When using `--get`, and the requested variable is not found, behave as if\n+ were the value assigned to the that variable.\n+\n CONFIGURATION\n -------------\n `pager.config` is only respected when listing configuration, i.e., when\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 9eda01615b..4f222a0ca9 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -26,6 +26,7 @@ static char term = '\\n';\n static int use_global_config, use_system_config, use_local_config;\n static struct git_config_source given_config_source;\n static int actions, type;\n+static char *default_value;\n static int end_null;\n static int respect_includes_opt = -1;\n static struct config_options config_options;\n@@ -148,6 +149,7 @@ static struct option builtin_config_options[] = {\n \tOPT_BOOL(0, \"name-only\", &omit_values, N_(\"show variable names only\")),\n \tOPT_BOOL(0, \"includes\", &respect_includes_opt, N_(\"respect include directives on lookup\")),\n \tOPT_BOOL(0, \"show-origin\", &show_origin, N_(\"show origin of config (file, standard input, blob, command line)\")),\n+\tOPT_STRING(0, \"default\", &default_value, N_(\"value\"), N_(\"with --get, use default value when missing entry\")),\n \tOPT_END(),\n };\n \n@@ -312,6 +314,16 @@ static int get_value(const char *key_, const char *regex_)\n \tconfig_with_options(collect_config, &values,\n \t\t\t &given_config_source, &config_options);\n \n+\tif (!values.nr && default_value) {\n+\t\tstruct strbuf *item;\n+\t\tALLOC_GROW(values.items, values.nr + 1, values.alloc);\n+\t\titem = &values.items[values.nr++];\n+\t\tstrbuf_init(item, 0);\n+\t\tif (format_config(item, key_, default_value) < 0)\n+\t\t\tdie(_(\"failed to format default config value: %s\"),\n+\t\t\t\tdefault_value);\n+\t}\n+\n \tret = !values.nr;\n \n \tfor (i = 0; i < values.nr; i++) {\n@@ -650,6 +662,12 @@ int cmd_config(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(builtin_config_usage, builtin_config_options);\n \t}\n \n+\tif (default_value && !(actions & ACTION_GET)) {\n+\t\terror(\"--default is only applicable to --get\");\n+\t\tusage_with_options(builtin_config_usage,\n+\t\t\tbuiltin_config_options);\n+\t}\n+\n \tif (actions & PAGING_ACTIONS)\n \t\tsetup_auto_pager(\"config\", 1);\n \ndiff --git a/t/t1310-config-default.sh b/t/t1310-config-default.sh\nnew file mode 100755\nindex 0000000000..6049d91708\n--- /dev/null\n+++ b/t/t1310-config-default.sh\n@@ -0,0 +1,36 @@\n+#!/bin/sh\n+\n+test_description='Test git config in different settings (with --default)'\n+\n+. ./test-lib.sh\n+\n+test_expect_success 'uses --default when entry missing' '\n+\techo quux >expect &&\n+\tgit config -f config --default=quux core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'does not use --default when entry present' '\n+\techo bar >expect &&\n+\tgit -c core.foo=bar config --default=baz core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'canonicalizes --default with appropriate type' '\n+\techo true >expect &&\n+\tgit config -f config --default=yes --bool core.foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'dies when --default cannot be parsed' '\n+\ttest_must_fail git config -f config --type=expiry-date --default=x --get \\\n+\t\tnot.a.section 2>error &&\n+\ttest_i18ngrep \"failed to format default config value\" error\n+'\n+\n+test_expect_success 'does not allow --default without --get' '\n+\ttest_must_fail git config --default=quux --unset a.section >output 2>&1 &&\n+\ttest_i18ngrep \"\\-\\-default is only applicable to\" output\n+'\n+\n+test_done\n-- \n2.17.0\n\n"},{"id":"345871","messageId":"bb9a3e13d26c3986dc91fea8e73a879602701b1e.1524722247.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524722247.git.me@ttaylorr.com","subject":"[PATCH v2 4/5] config.c: introduce 'git_config_color' to parse ANSI colors","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T05:58:36Z","receivedAt":"2018-04-26T05:58:47Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"In preparation for adding `--type=color` to the `git-config(1)` builtin,\nlet's introduce a color parsing utility, `git_config_color` in a similar\nfashion to `git_config_`.\n\nSigned-off-by: Taylor Blau \n---\n config.c | 10 ++++++++++\n config.h | 1 +\n 2 files changed, 11 insertions(+)\n\ndiff --git a/config.c b/config.c\nindex 62c56099bf..d14813bef7 100644\n--- a/config.c\n+++ b/config.c\n@@ -16,6 +16,7 @@\n #include \"string-list.h\"\n #include \"utf8.h\"\n #include \"dir.h\"\n+#include \"color.h\"\n \n struct config_source {\n \tstruct config_source *prev;\n@@ -1000,6 +1001,15 @@ int git_config_expiry_date(timestamp_t *timestamp, const char *var, const char *\n \treturn 0;\n }\n \n+int git_config_color(char *dest, const char *var, const char *value)\n+{\n+\tif (!value)\n+\t\treturn config_error_nonbool(var);\n+\tif (color_parse(value, dest) < 0)\n+\t\treturn -1;\n+\treturn 0;\n+}\n+\n static int git_default_core_config(const char *var, const char *value)\n {\n \t/* This needs a better name */\ndiff --git a/config.h b/config.h\nindex ef70a9cac1..0e060779d9 100644\n--- a/config.h\n+++ b/config.h\n@@ -59,6 +59,7 @@ extern int git_config_bool(const char *, const char *);\n extern int git_config_string(const char **, const char *, const char *);\n extern int git_config_pathname(const char **, const char *, const char *);\n extern int git_config_expiry_date(timestamp_t *, const char *, const char *);\n+extern int git_config_color(char *, const char *, const char *);\n extern int git_config_set_in_file_gently(const char *, const char *, const char *);\n extern void git_config_set_in_file(const char *, const char *, const char *);\n extern int git_config_set_gently(const char *, const char *);\n-- \n2.17.0\n\n"},{"id":"345872","messageId":"d9a737fde37e778c637de67e1215960f467c629d.1524722247.git.me@ttaylorr.com","threadId":"48361","inReplyTo":"cover.1524722247.git.me@ttaylorr.com","subject":"[PATCH v2 5/5] builtin/config: introduce `color` type specifier","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T05:58:39Z","receivedAt":"2018-04-26T05:59:02Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"As of this commit, the canonical way to retreive an ANSI-compatible\ncolor escape sequence from a configuration file is with the\n`--get-color` action.\n\nThis is to allow Git to \"fall back\" on a default value for the color\nshould the given section not exist in the specified configuration(s).\n\nWith the addition of `--default`, this is no longer needed since:\n\n $ git config --default red --type=color core.section\n\nwill be have exactly as:\n\n $ git config --get-color core.section red\n\nFor consistency, let's introduce `--type=color` and encourage its use\nwith `--default` together over `--get-color` alone.\n\nSigned-off-by: Taylor Blau \n---\n Documentation/git-config.txt | 6 ++++++\n builtin/config.c | 22 ++++++++++++++++++++++\n t/t1300-repo-config.sh | 30 ++++++++++++++++++++++++++++++\n 3 files changed, 58 insertions(+)\n\ndiff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\nindex c3adafd78a..18ddc78f42 100644\n--- a/Documentation/git-config.txt\n+++ b/Documentation/git-config.txt\n@@ -177,6 +177,10 @@ Valid ``'s include:\n ~/` from the command line to let your shell do the expansion.)\n - 'expiry-date': canonicalize by converting from a fixed or relative date-string\n to a timestamp. This specifier has no effect when setting the value.\n+- 'color': When getting a value, canonicalize by converting to an ANSI color\n+ escape sequence. When setting a value, a sanity-check is performed to ensure\n+ that the given value is canonicalize-able as an ANSI color, but it is written\n+ as-is.\n +\n \n --bool::\n@@ -228,6 +232,8 @@ Valid ``'s include:\n \toutput it as the ANSI color escape sequence to the standard\n \toutput. The optional `default` parameter is used instead, if\n \tthere is no color configured for `name`.\n++\n+`--type=color [--default=]` is preferred over `--get-color`.\n \n -e::\n --edit::\ndiff --git a/builtin/config.c b/builtin/config.c\nindex 4f222a0ca9..bb62816bba 100644\n--- a/builtin/config.c\n+++ b/builtin/config.c\n@@ -61,6 +61,7 @@ static int show_origin;\n #define TYPE_BOOL_OR_INT\t3\n #define TYPE_PATH\t\t4\n #define TYPE_EXPIRY_DATE\t5\n+#define TYPE_COLOR\t\t6\n \n #define OPT_CALLBACK_VALUE(s, l, v, h, i) \\\n \t{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \\\n@@ -94,6 +95,8 @@ static int option_parse_type(const struct option *opt, const char *arg,\n \t\t\tnew_type = TYPE_PATH;\n \t\telse if (!strcmp(arg, \"expiry-date\"))\n \t\t\tnew_type = TYPE_EXPIRY_DATE;\n+\t\telse if (!strcmp(arg, \"color\"))\n+\t\t\tnew_type = TYPE_COLOR;\n \t\telse\n \t\t\tdie(_(\"unrecognized --type argument, %s\"), arg);\n \t}\n@@ -229,6 +232,11 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value\n \t\t\tif (git_config_expiry_date(&t, key_, value_) < 0)\n \t\t\t\treturn -1;\n \t\t\tstrbuf_addf(buf, \"%\"PRItime, t);\n+\t\t} else if (type == TYPE_COLOR) {\n+\t\t\tchar v[COLOR_MAXLEN];\n+\t\t\tif (git_config_color(v, key_, value_) < 0)\n+\t\t\t\treturn -1;\n+\t\t\tstrbuf_addstr(buf, v);\n \t\t} else if (value_) {\n \t\t\tstrbuf_addstr(buf, value_);\n \t\t} else {\n@@ -374,6 +382,20 @@ static char *normalize_value(const char *key, const char *value)\n \t\telse\n \t\t\treturn xstrdup(v ? \"true\" : \"false\");\n \t}\n+\tif (type == TYPE_COLOR) {\n+\t\tchar v[COLOR_MAXLEN];\n+\t\tif (git_config_color(v, key, value))\n+\t\t\tdie(\"cannot parse color '%s'\", value);\n+\n+\t\t/*\n+\t\t * The contents of `v` now contain an ANSI escape\n+\t\t * sequence, not suitable for including within a\n+\t\t * configuration file. Treat the above as a\n+\t\t * \"sanity-check\", and return the given value, which we\n+\t\t * know is representable as valid color code.\n+\t\t */\n+\t\treturn xstrdup(value);\n+\t}\n \n \tdie(\"BUG: cannot normalize type %d\", type);\n }\ndiff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\nindex 1e3a1ace14..2acfd513f1 100755\n--- a/t/t1300-repo-config.sh\n+++ b/t/t1300-repo-config.sh\n@@ -931,6 +931,36 @@ test_expect_success 'get --expiry-date' '\n \ttest_must_fail git config --expiry-date date.invalid1\n '\n \n+test_expect_success 'get --type=color' '\n+\trm .git/config &&\n+\tgit config foo.color \"red\" &&\n+\tgit config --get --type=color foo.color >actual.raw &&\n+\ttest_decode_color actual &&\n+\techo \"\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+cat >expect << EOF\n+[foo]\n+\tcolor = red\n+EOF\n+\n+test_expect_success 'set --type=color' '\n+\trm .git/config &&\n+\tgit config --type=color foo.color \"red\" &&\n+\ttest_cmp expect .git/config\n+'\n+\n+test_expect_success 'get --type=color barfs on non-color' '\n+\techo \"[foo]bar=not-a-color\" >.git/config &&\n+\ttest_must_fail git config --get --type=color foo.bar\n+'\n+\n+test_expect_success 'set --type=color barfs on non-color' '\n+\ttest_must_fail git config --type=color foo.color \"not-a-color\" 2>error &&\n+\ttest_i18ngrep \"cannot parse color\" error\n+'\n+\n cat > expect << EOF\n [quote]\n \tleading = \" test\"\n-- \n2.17.0\n"},{"id":"345873","messageId":"20180426060023.GA39448@syl.local","threadId":"48361","inReplyTo":"xmqq7eou35ev.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T06:00:23Z","receivedAt":"2018-04-26T06:00:28Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Apr 26, 2018 at 02:25:44PM +0900, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`\n>\n> I'd retitle while queuing, as the last 'type' is a placeholder for\n> concrete types like above.\n\nGood idea. I amended v2 in this fashion.\n\n> > +...\n> > +\tnew_type = opt->defval;\n> > +\tif (!new_type) {\n> > +...\n> > +\t}\n> > +\n> > +\t*to_type = opt->value;\n>\n> But this is wrong, no? You meant opt->value points at an integer\n> variable that receives the type we discover by parsing, i.e.\n>\n> \tto_type = opt->value;\n\nOof. You're absolutely right. I fixed this and moved the assignment to\nthe declaration at the top of this function.\n\n\nThanks,\nTaylor\n"},{"id":"345874","messageId":"20180426060115.GA40265@syl.local","threadId":"48361","inReplyTo":"xmqq36zi352x.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 5/5] builtin/config: introduce `color` type specifier","fromName":"Taylor Blau","fromEmail":"me@ttaylorr.com","sentAt":"2018-04-26T06:01:15Z","receivedAt":"2018-04-26T06:01:22Z","isPatch":true,"sender":{"key":"me@ttaylorr.com","avatar":"https://avatars.githubusercontent.com/u/301000140?v=4"},"body":"On Thu, Apr 26, 2018 at 02:32:54PM +0900, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > diff --git a/builtin/config.c b/builtin/config.c\n> > index d7acf912cd..ec5c11293b 100644\n> > --- a/builtin/config.c\n> > +++ b/builtin/config.c\n> > @@ -61,6 +61,7 @@ static int show_origin;\n> > #define TYPE_BOOL_OR_INT\t3\n> > #define TYPE_PATH\t\t4\n> > #define TYPE_EXPIRY_DATE\t5\n> > +#define TYPE_COLOR\t\t6\n> >\n> > #define OPT_CALLBACK_VALUE(s, l, v, h, i) \\\n> > \t{ OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \\\n> > @@ -231,6 +232,11 @@ static int format_config(struct strbuf *buf, const char *key_, const char *value\n> > \t\t\tif (git_config_expiry_date(&t, key_, value_) < 0)\n> > \t\t\t\treturn -1;\n> > \t\t\tstrbuf_addf(buf, \"%\"PRItime, t);\n> > +\t\t} else if (type == TYPE_COLOR) {\n> > +\t\t\tchar v[COLOR_MAXLEN];\n> > +\t\t\tif (git_config_color(v, key_, value_) < 0)\n> > +\t\t\t\treturn -1;\n> > +\t\t\tstrbuf_addstr(buf, v);\n> > \t\t} else if (value_) {\n> > \t\t\tstrbuf_addstr(buf, value_);\n> > \t\t} else {\n> > @@ -376,6 +382,20 @@ static char *normalize_value(const char *key, const char *value)\n> > \t\telse\n> > \t\t\treturn xstrdup(v ? \"true\" : \"false\");\n> > \t}\n> > +\tif (type == TYPE_COLOR) {\n> > +\t\tchar v[COLOR_MAXLEN];\n> > +\t\tif (git_config_color(v, key, value))\n> > +\t\t\tdie(\"cannot parse color '%s'\", value);\n> > +\n> > +\t\t/*\n> > +\t\t * The contents of `v` now contain an ANSI escape\n> > +\t\t * sequence, not suitable for including within a\n> > +\t\t * configuration file. Treat the above as a\n> > +\t\t * \"sanity-check\", and return the given value, which we\n> > +\t\t * know is representable as valid color code.\n> > +\t\t */\n> > +\t\treturn xstrdup(value);\n> > +\t}\n> >\n> > \tdie(\"BUG: cannot normalize type %d\", type);\n> > }\n>\n> Hmmm, option_parse_type() introduced in [PATCH 2/5] used to learn\n> to parse \"color\" in this step, but it no longer does.\n\nOof, again. I dropped this hunk on the floor when integrating. I put it\nback in v2.\n\n\nThanks,\nTaylor\n"},{"id":"345890","messageId":"xmqqmuxq1emd.fsf@gitster-ct.c.googlers.com","threadId":"48361","inReplyTo":"404da8bdeb9808d2dfcfdf09bc62c76195c9a08c.1524722247.git.me@ttaylorr.com","subject":"Re: [PATCH v2 2/5] builtin/config.c: support `--type=` as preferred alias for `--`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-04-26T09:49:46Z","receivedAt":"2018-04-26T09:55:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Taylor Blau writes:\n\n> +static int option_parse_type(const struct option *opt, const char *arg,\n> +\t\t\t int unset)\n> +{\n> +\t/*\n> +\t * To support '--' style flags, begin with new_type equal to\n> +\t * opt->defval.\n> +\t */\n> +\tint new_type = opt->defval;\n> +\tint *to_type = opt->value;\n> +\n> +\tif (unset) {\n> +\t\t*((int *) opt->value) = 0;\n\nAs you moved the definition of to_type higher than the previous\nrounds, you can already use it here to avoid cryptic casting, i.e.\n\n\t\t*to_type = 0;\n\nno?\n\n> +\t\treturn 0;\n> +\t}\n"},{"id":"345938","messageId":"CAPig+cQticAS2wrucAVkxiEuGuwoDeyvXKdGb=7s4m+iugYmCA@mail.gmail.com","threadId":"48361","inReplyTo":"404da8bdeb9808d2dfcfdf09bc62c76195c9a08c.1524722247.git.me@ttaylorr.com","subject":"Re: [PATCH v2 2/5] builtin/config.c: support `--type=` as preferred alias for `--`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-27T05:57:14Z","receivedAt":"2018-04-27T05:57:18Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 26, 2018 at 1:58 AM, Taylor Blau wrote:\n> `git config` has long allowed the ability for callers to provide a 'type\n> specifier', which instructs `git config` to (1) ensure that incoming\n> values can be interpreted as that type, and (2) that outgoing values are\n> canonicalized under that type.\n>\n> In another series, we propose to extend this functionality with\n> `--type=color` and `--default` to replace `--get-color`.\n\nNow that you've combined the two series, this sentence no longer makes\nsense as written. Perhaps say:\n\n Later patches will extend this functionality...\n\n> However, we traditionally use `--color` to mean \"colorize this output\",\n> instead of \"this value should be treated as a color\".\n>\n> Currently, `git config` does not support this kind of colorization, but\n> we should be careful to avoid squatting on this option too soon, so that\n> `git config` can support `--color` (in the traditional sense) in the\n> future, if that is desired.\n>\n> In this patch, we support `--type=` in\n> addition to `--int`, `--bool`, and etc. This allows the aforementioned\n> upcoming patch to support querying a color value with a default via\n> `--type=color --default=...`, without squandering `--color`.\n>\n> We retain the historic behavior of complaining when multiple,\n\nDrop the comma and be more specific:\ns/multiple,/multiple conflicting/\n\n> legacy-style `--` flags are given, as well as extend this to\n> conflicting new-style `--type=` flags. `--int --type=int` (and its\n> commutative pair) does not complain, but `--bool --type=int` (and its\n> commutative pair) does.\n\nConfusing. Part of the selling point of the commit message of patch\n1/5 is the removal of this complaint/restriction, claiming that it\nintentionally treats \"git config --int --bool\" simply as \"git config\n--bool\", and that that loosening is required to support \"git config\n--int --type=int\" without complaining, thus is a good thing. But this\ncommit message (2/5) backpedals and reinstates the original\ncomplaint/restriction.\n\nPerhaps I could have understood if 1/5 said that the loosening of the\nrestriction was only temporary and that it would be restored by a\nlater patch rather than using the restriction-removal as a selling\npoint. However, this patch series doesn't need to be crafted such that\na feature is temporarily lost and later restored, so I'm having\ntrouble buying the way this series is architected.\n\nWhat could make more sense would be for 1/5 to introduce\noption_parse_type() for --, thus retaining the restriction, and\nfor 2/5 simply to augment option_parse_type() to also understand\n--type=.\n\n> Signed-off-by: Taylor Blau \n"},{"id":"345939","messageId":"CAPig+cQwwsLSRUDqvM2LSv-DJDJQwv+MY5ANVhEiv6oBpXuvHw@mail.gmail.com","threadId":"48361","inReplyTo":"20180426060023.GA39448@syl.local","subject":"Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-27T06:06:01Z","receivedAt":"2018-04-27T06:06:05Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 26, 2018 at 2:00 AM, Taylor Blau wrote:\n> On Thu, Apr 26, 2018 at 02:25:44PM +0900, Junio C Hamano wrote:\n>> Taylor Blau writes:\n>>\n>> > Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`\n>>\n>> I'd retitle while queuing, as the last 'type' is a placeholder for\n>> concrete types like above.\n>\n> Good idea. I amended v2 in this fashion.\n\nSuggested previously[1], as well.\n\n[1]: https://public-inbox.org/git/CAPig+cTw5ZXKGOcnombwiM4P+9iRpbYUvOagQ0XujLB0ttXsOg@mail.gmail.com/\n"},{"id":"345940","messageId":"CAPig+cQx3RKCyAisVdEiwLpVPEhPRVWJ2rdA8yawZFq1891mwg@mail.gmail.com","threadId":"48361","inReplyTo":"d9a737fde37e778c637de67e1215960f467c629d.1524722247.git.me@ttaylorr.com","subject":"Re: [PATCH v2 5/5] builtin/config: introduce `color` type specifier","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2018-04-27T06:22:23Z","receivedAt":"2018-04-27T06:22:27Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Thu, Apr 26, 2018 at 1:58 AM, Taylor Blau wrote:\n> [...]\n> For consistency, let's introduce `--type=color` and encourage its use\n> with `--default` together over `--get-color` alone.\n>\n> Signed-off-by: Taylor Blau \n> ---\n> diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt\n> @@ -228,6 +232,8 @@ Valid ``'s include:\n> output it as the ANSI color escape sequence to the standard\n> output. The optional `default` parameter is used instead, if\n> there is no color configured for `name`.\n> ++\n> +`--type=color [--default=]` is preferred over `--get-color`.\n\nRather than augmenting the documentation of --get-color like this, I\nwonder if it would instead make more sense to replace it entirely,\nlike this:\n\n --get-color name []\n Deprecated alias for `--type=color [--default=]`.\n\nNot necessarily worth a re-roll; could be done as a follow-up if\nsomeone cares enough.\n\n> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh\n> @@ -931,6 +931,36 @@ test_expect_success 'get --expiry-date' '\n> +test_expect_success 'get --type=color' '\n> + rm .git/config &&\n\nI would feel more confident about the robustness of this test if it\ninstead used 'rm -f .git/config', as is already done elsewhere in this\ntest script.\n\n> + git config foo.color \"red\" &&\n> + git config --get --type=color foo.color >actual.raw &&\n> + test_decode_color actual &&\n> + echo \"\" >expect &&\n> + test_cmp expect actual\n> +'\n> +\n> +test_expect_success 'set --type=color' '\n> + rm .git/config &&\n\nDitto.\n\n> + git config --type=color foo.color \"red\" &&\n> + test_cmp expect .git/config\n> +'\n"}]}