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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Taylor Blau <me@xxxxxxxxxxxx> 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?

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

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux