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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2018, 01:44 UTC
Message-ID: <xmqqsh834yz1.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <20180409224657.GC34547@syl.local>

Taylor Blau <me@ttaylorr.com> writes:

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

Hmm, "satisfiable" is not the first word that comes to my mind to describe "when you give me a string 'frotz', we cannot take it as an integer". s/are satisfiable under/can be interpreted as/ perhaps?

Show 5 quoted lines

> 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".

Makes sense.

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

Shouldn't the above `--color` instead? That is, we avoid squatting on `--color` because we might later want to use it in the traditional way. Also my reading stuttered around "inhabiting". It might be just me, but perhaps s/inhabiting/squating on/?

> 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`.

Good.

Show 17 quoted lines

> @@ -160,30 +158,39 @@ See also <<FILES>>.
>  --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 prior to output.

It is not a new problem introduced by this patch, but these place too much weight on the output side and may end up giving a flawed mental model to the readers, I am afraid.

It's not like an 'int' value internally is "2k" and shown it as 2048 when output. In reality, we internalize it 2048 upon input and we do not do anything special when showing, no? A 'bool' value given with a string 'no' internally becomes 0 upon input and is shown as 'false' upon output.

I suspect s/prior to output/upon input/ may alleviate the issue quite a bit.