Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`

[Eric Sunshine]

On Thu, Apr 26, 2018 at 2:00 AM, Taylor Blau <m...@ttaylorr.com> wrote:
> On Thu, Apr 26, 2018 at 02:25:44PM +0900, Junio C Hamano wrote:
>> Taylor Blau <m...@ttaylorr.com> writes:
>>
>> > Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=` as 
>> > preferred alias for `--type`
>>
>> I'd retitle while queuing, as the last 'type' is a placeholder for
>> concrete types like  above.
>
> Good idea. I amended v2 in this fashion.

Suggested previously[1], as well.

[1]: 
https://public-inbox.org/git/capig+ctw5zxkgocnombwim4p+9irpbyuvoagq0xujlb0ttx...@mail.gmail.com/

Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`

[Taylor Blau]

On Thu, Apr 26, 2018 at 02:25:44PM +0900, Junio C Hamano wrote:
> Taylor Blau <m...@ttaylorr.com> writes:
>
> > Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=` as 
> > preferred alias for `--type`
>
> I'd retitle while queuing, as the last 'type' is a placeholder for
> concrete types like  above.

Good idea. I amended v2 in this fashion.

> > +...
> > +   new_type = opt->defval;
> > +   if (!new_type) {
> > +...
> > +   }
> > +
> > +   *to_type = opt->value;
>
> But this is wrong, no?  You meant opt->value points at an integer
> variable that receives the type we discover by parsing, i.e.
>
>   to_type = opt->value;

Oof. You're absolutely right. I fixed this and moved the assignment to
the declaration at the top of this function.


Thanks,
Taylor

Re: [PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`

[Junio C Hamano]

Taylor Blau <m...@ttaylorr.com> writes:

> Subject: Re: [PATCH 2/5] builtin/config.c: support `--type=` as 
> preferred alias for `--type`

I'd retitle while queuing, as the last 'type' is a placeholder for
concrete types like  above.

> +static int option_parse_type(const struct option *opt, const char *arg,
> +  int unset)
> +{
> + int new_type;
> + int *to_type;

Splitting these into two lines (unlike placing on a single same
line, which was how the previous round was queued) like this is
good.

> +...
> + new_type = opt->defval;
> + if (!new_type) {
> +...
> + }
> +
> + *to_type = opt->value;

But this is wrong, no?  You meant opt->value points at an integer
variable that receives the type we discover by parsing, i.e.

to_type = opt->value;

[PATCH 2/5] builtin/config.c: support `--type=` as preferred alias for `--type`

[Taylor Blau]

`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=` 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 `--` flags are given, as well as extend this to
conflicting new-style `--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 
---
 Documentation/git-config.txt | 71 
 builtin/config.c | 66 ++---
 t/t1300-repo-config.sh   | 58 +++--
 3 files changed, 155 insertions(+), 40 deletions(-)

diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
index e09ed5d7d5..b759009c75 100644
--- 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]
-'git config' [] [type] [--show-origin] [-z|--null] name [value 
[value_regex]]
-'git config' [] [type] --add name value
-'git config' [] [type] --replace-all name value [value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] --get-all name 
[value_regex]
-'git config' [] [type] [--show-origin] [-z|--null] [--name-only] 
--get-regexp name_regex [value_regex]
-'git config' [] [type] [-z|--null] --get-urlmatch name URL
+'git config' [] [--type=] [--show-origin] [-z|--null] name 
[value [value_regex]]
+'git config' [] [--type=] --add name value
+'git config' [] [--type=] --replace-all name value 
[value_regex]
+'git config' [] [--type=] [--show-origin] [-z|--null] --get 
name [value_regex]
+'git config' [] [--type=] [--show-origin] [-z|--null] 
--get-all name [value_regex]
+'git config' [] [--type=] [--show-origin] [-z|--null] 
[--name-only] --get-regexp name_regex [value_regex]
+'git config' [] [--type=] [-z|--null] --get-urlmatch name 
URL
 'git config' [] --unset name [value_regex]
 'git config' [] --unset-all name [value_regex]
 'git config' [] --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 <>).
 
-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=` option instructs 'git config' to ensure that incoming and
+outgoing values are canonicalize-able under the given .  If no
+`--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
@@ -160,30 +158,39 @@ See also <>.
 --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 ::
+  'git config' will ensure that any input or output is valid under the given
+  type constraint(s), and will canonicalize outgoing values in ``'s
+  canonical form.
++
+Valid ``'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