builtin/config.c: support `--type=<type>` as preferred alias for `--<type>`

`git config` has long allowed the ability for callers to provide a 'type
specifier', which instructs `git config` to (1) ensure that incoming
values can be interpreted as that type, and (2) that outgoing values are
canonicalized under that type.

In another series, we propose to extend this functionality with
`--type=color` and `--default` to replace `--get-color`.

However, we traditionally use `--color` to mean "colorize this output",
instead of "this value should be treated as a color".

Currently, `git config` does not support this kind of colorization, but
we should be careful to avoid squatting on this option too soon, so that
`git config` can support `--color` (in the traditional sense) in the
future, if that is desired.

In this patch, we support `--type=<int|bool|bool-or-int|...>` in
addition to `--int`, `--bool`, and etc. This allows the aforementioned
upcoming patch to support querying a color value with a default via
`--type=color --default=...`, without squandering `--color`.

We retain the historic behavior of complaining when multiple,
legacy-style `--<type>` flags are given, as well as extend this to
conflicting new-style `--type=<type>` flags. `--int --type=int` (and its
commutative pair) does not complain, but `--bool --type=int` (and its
commutative pair) does.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e09ed5d7d5147d93039c479efc8ab450bf5ca8b4..b759009c7571ee79b39a4ef12750eaff311f61c0 100644 (file)
--- a/Documentation/git-config.txt
+++ b/Documentation/git-config.txt
@@ -9,13 +9,13 @@ git-config - Get and set repository or global options
 SYNOPSIS
 --------
 [verse]
 SYNOPSIS
 --------
 [verse]

-'git config' [<file-option>] [type] [--show-origin] [-z|--null] name [value [value_regex]]
-'git config' [<file-option>] [type] --add name value
-'git config' [<file-option>] [type] --replace-all name value [value_regex]
-'git config' [<file-option>] [type] [--show-origin] [-z|--null] --get name [value_regex]
-'git config' [<file-option>] [type] [--show-origin] [-z|--null] --get-all name [value_regex]
-'git config' [<file-option>] [type] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
-'git config' [<file-option>] [type] [-z|--null] --get-urlmatch name URL
+'git config' [<file-option>] [--type=<type>] [--show-origin] [-z|--null] name [value [value_regex]]
+'git config' [<file-option>] [--type=<type>] --add name value
+'git config' [<file-option>] [--type=<type>] --replace-all name value [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [-z|--null] --get name [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [-z|--null] --get-all name [value_regex]
+'git config' [<file-option>] [--type=<type>] [--show-origin] [-z|--null] [--name-only] --get-regexp name_regex [value_regex]
+'git config' [<file-option>] [--type=<type>] [-z|--null] --get-urlmatch name URL

 'git config' [<file-option>] --unset name [value_regex]
 'git config' [<file-option>] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name
 'git config' [<file-option>] --unset name [value_regex]
 'git config' [<file-option>] --unset-all name [value_regex]
 'git config' [<file-option>] --rename-section old_name new_name


@@ -38,12 +38,10 @@ existing values that match the regexp are updated or unset.  If
 you want to handle the lines that do *not* match the regex, just
 prepend a single exclamation mark in front (see also <<EXAMPLES>>).
 
 you want to handle the lines that do *not* match the regex, just
 prepend a single exclamation mark in front (see also <<EXAMPLES>>).
 

-The type specifier can be either `--int` or `--bool`, to make
-'git config' ensure that the variable(s) are of the given type and
-convert the value to the canonical form (simple decimal number for int,
-a "true" or "false" string for bool), or `--path`, which does some
-path expansion (see `--path` below).  If no type specifier is passed, no
-checks or transformations are performed on the value.
+The `--type=<type>` option instructs 'git config' to ensure that incoming and
+outgoing values are canonicalize-able under the given <type>.  If no
+`--type=<type>` is given, no canonicalization will be performed. Callers may
+unset an existing `--type` specifier with `--no-type`.

 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options
 
 When reading, the values are read from the system, global and
 repository local configuration files by default, and options


@@ -160,30 +158,39 @@ See also <<FILES>>.
 --list::
        List all variables set in config file, along with their values.
 
 --list::
        List all variables set in config file, along with their values.
 

---bool::
-       'git config' will ensure that the output is "true" or "false"
+--type <type>::
+  'git config' will ensure that any input or output is valid under the given
+  type constraint(s), and will canonicalize outgoing values in `<type>`'s
+  canonical form.
++
+Valid `<type>`'s include:
++
+- 'bool': canonicalize values as either "true" or "false".
+- 'int': canonicalize values as simple decimal numbers. An optional suffix of
+  'k', 'm', or 'g' will cause the value to be multiplied by 1024, 1048576, or
+  1073741824 upon input.
+- 'bool-or-int': canonicalize according to either 'bool' or 'int', as described
+  above.
+- 'path': canonicalize by adding a leading `~` to the value of `$HOME` and
+  `~user` to the home directory for the specified user. This specifier has no
+  effect when setting the value (but you can use `git config section.variable
+  ~/` from the command line to let your shell do the expansion.)
+- 'expiry-date': canonicalize by converting from a fixed or relative date-string
+  to a timestamp. This specifier has no effect when setting the value.
++

 
 

+--bool::

 --int::
 --int::

-       'git config' will ensure that the output is a simple
-       decimal number.  An optional value suffix of 'k', 'm', or 'g'
-       in the config file will cause the value to be multiplied
-       by 1024, 1048576, or 1073741824 prior to output.
-

 --bool-or-int::
 --bool-or-int::

-       'git config' will ensure that the output matches the format of
-       either --bool or --int, as described above.
-

 --path::
 --path::

-       `git config` will expand a leading `~` to the value of
-       `$HOME`, and `~user` to the home directory for the
-       specified user.  This option has no effect when setting the
-       value (but you can use `git config section.variable ~/`
-       from the command line to let your shell do the expansion).
-

 --expiry-date::
 --expiry-date::

-       `git config` will ensure that the output is converted from
-       a fixed or relative date-string to a timestamp. This option
-       has no effect when setting the value.
+  Historical options for selecting a type specifier. Prefer instead `--type`,
+  (see: above).
+
+--no-type::
+  Un-sets the previously set type specifier (if one was previously set). This
+  option requests that 'git config' not canonicalize the retrieved variable.
+  `--no-type` has no effect without `--type=<type>` or `--<type>`.

 
 -z::
 --null::
 
 -z::
 --null::

diff --git a/builtin/config.c b/builtin/config.c
index 92fb8d56b11e0be5d781f91aa1e5b1fcae7002a7..24d735e16a390ca54577199423c773f4028885a3 100644 (file)
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -61,6 +61,60 @@ static int show_origin;
 #define TYPE_PATH              4
 #define TYPE_EXPIRY_DATE       5
 
 #define TYPE_PATH              4
 #define TYPE_EXPIRY_DATE       5
 

+#define OPT_CALLBACK_VALUE(s, l, v, h, i) \
+       { OPTION_CALLBACK, (s), (l), (v), NULL, (h), PARSE_OPT_NOARG | \
+       PARSE_OPT_NONEG, option_parse_type, (i) }
+
+static struct option builtin_config_options[];
+
+static int option_parse_type(const struct option *opt, const char *arg,
+                            int unset)
+{
+       int new_type, *to_type;
+
+       if (unset) {
+               *((int *) opt->value) = 0;
+               return 0;
+       }
+
+       /*
+        * To support '--<type>' style flags, begin with new_type equal to
+        * opt->defval.
+        */
+       new_type = opt->defval;
+       if (!new_type) {
+               if (!strcmp(arg, "bool"))
+                       new_type = TYPE_BOOL;
+               else if (!strcmp(arg, "int"))
+                       new_type = TYPE_INT;
+               else if (!strcmp(arg, "bool-or-int"))
+                       new_type = TYPE_BOOL_OR_INT;
+               else if (!strcmp(arg, "path"))
+                       new_type = TYPE_PATH;
+               else if (!strcmp(arg, "expiry-date"))
+                       new_type = TYPE_EXPIRY_DATE;
+               else
+                       die(_("unrecognized --type argument, %s"), arg);
+       }
+
+       to_type = opt->value;
+       if (*to_type && *to_type != new_type) {
+               /*
+                * Complain when there is a new type not equal to the old type.
+                * This allows for combinations like '--int --type=int' and
+                * '--type=int --type=int', but disallows ones like '--type=bool
+                * --int' and '--type=bool
+                * --type=int'.
+                */
+               error("only one type at a time.");
+               usage_with_options(builtin_config_usage,
+                       builtin_config_options);
+       }
+       *to_type = new_type;
+
+       return 0;
+}
+

 static struct option builtin_config_options[] = {
        OPT_GROUP(N_("Config file location")),
        OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),
 static struct option builtin_config_options[] = {
        OPT_GROUP(N_("Config file location")),
        OPT_BOOL(0, "global", &use_global_config, N_("use global config file")),


@@ -84,11 +138,12 @@ static struct option builtin_config_options[] = {
        OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
        OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
        OPT_GROUP(N_("Type")),
        OPT_BIT(0, "get-color", &actions, N_("find the color configured: slot [default]"), ACTION_GET_COLOR),
        OPT_BIT(0, "get-colorbool", &actions, N_("find the color setting: slot [stdout-is-tty]"), ACTION_GET_COLORBOOL),
        OPT_GROUP(N_("Type")),

-       OPT_SET_INT(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL),
-       OPT_SET_INT(0, "int", &type, N_("value is decimal number"), TYPE_INT),
-       OPT_SET_INT(0, "bool-or-int", &type, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
-       OPT_SET_INT(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
-       OPT_SET_INT(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),
+       OPT_CALLBACK('t', "type", &type, "", N_("value is given this type"), option_parse_type),
+       OPT_CALLBACK_VALUE(0, "bool", &type, N_("value is \"true\" or \"false\""), TYPE_BOOL),
+       OPT_CALLBACK_VALUE(0, "int", &type, N_("value is decimal number"), TYPE_INT),
+       OPT_CALLBACK_VALUE(0, "bool-or-int", &type, N_("value is --bool or --int"), TYPE_BOOL_OR_INT),
+       OPT_CALLBACK_VALUE(0, "path", &type, N_("value is a path (file or directory name)"), TYPE_PATH),
+       OPT_CALLBACK_VALUE(0, "expiry-date", &type, N_("value is an expiry date"), TYPE_EXPIRY_DATE),

        OPT_GROUP(N_("Other")),
        OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
        OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),
        OPT_GROUP(N_("Other")),
        OPT_BOOL('z', "null", &end_null, N_("terminate values with NUL byte")),
        OPT_BOOL(0, "name-only", &omit_values, N_("show variable names only")),

diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 24de37d5447c7def8a36213872b8bb369bb98458..e06af3d337ac037d799f878025d175bd1271a524 100755 (executable)
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1613,13 +1613,65 @@ test_expect_success '--local requires a repo' '
 
 cat >.git/config <<-\EOF &&
 [core]
 
 cat >.git/config <<-\EOF &&
 [core]

+foo = true

 number = 10
 number = 10

+big = 1M

 EOF
 
 EOF
 

-test_expect_success 'later legacy specifiers are given precedence' '
-       git config --bool --int core.number >actual &&
-       echo 10 >expect &&
+test_expect_success 'identical modern --type specifiers are allowed' '
+       git config --type=int --type=int core.big >actual &&
+       echo 1048576 >expect &&

        test_cmp expect actual
 '
 
        test_cmp expect actual
 '
 

+test_expect_success 'identical legacy --type specifiers are allowed' '
+       git config --int --int core.big >actual &&
+       echo 1048576 >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'identical mixed --type specifiers are allowed' '
+       git config --int --type=int core.big >actual &&
+       echo 1048576 >expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'non-identical modern --type specifiers are not allowed' '
+       test_must_fail git config --type=int --type=bool core.big 2>error &&
+       test_i18ngrep "only one type at a time" error
+'
+
+test_expect_success 'non-identical legacy --type specifiers are not allowed' '
+       test_must_fail git config --int --bool core.big 2>error &&
+       test_i18ngrep "only one type at a time" error
+'
+
+test_expect_success 'non-identical mixed --type specifiers are not allowed' '
+       test_must_fail git config --type=int --bool core.big 2>error &&
+       test_i18ngrep "only one type at a time" error
+'
+
+test_expect_success '--type allows valid type specifiers' '
+       echo "true" >expect &&
+       git config --type=bool core.foo >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--no-type unsets type specifiers' '
+       echo "10" >expect &&
+       git config --type=bool --no-type core.number >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success 'unset type specifiers may be reset to conflicting ones' '
+       echo 1048576 >expect &&
+       git config --type=bool --no-type --type=int core.big >actual &&
+       test_cmp expect actual
+'
+
+test_expect_success '--type rejects unknown specifiers' '
+       test_must_fail git config --type=nonsense core.foo 2>error &&
+       test_i18ngrep "unrecognized --type argument" error
+'
+

 test_done
 test_done