Re: [PATCH v5 3/3] builtin/config: introduce `color` type specifier

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

 



On Fri, Apr 06, 2018 at 03:29:48AM -0400, Eric Sunshine wrote:
> On Fri, Apr 6, 2018 at 2:30 AM, Taylor Blau <me@xxxxxxxxxxxx> wrote:
> > [...]
> > For consistency, let's introduce `--type=color` and encourage its use
> > with `--default` together over `--get-color` alone.
>
> A couple minor subjective comments below... neither worth a re-roll.
>
> > Signed-off-by: Taylor Blau <me@xxxxxxxxxxxx>
> > ---
> > diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt
> > @@ -180,6 +180,10 @@ Valid `<type>`'s include:
> > +- 'color': When writing to a caller, canonicalize by converting to an ANSI color
> > +  escape sequence. When writing to the configuration file, a sanity-check is
> > +  performed to ensure that the given value is canonicalize-able as an ANSI
> > +  color, but it is written as-is.
>
> "When writing to a caller" is a somewhat confusing way to say "When
> getting a value".
>
> Likewise, "When writing to the configuration file" could be stated
> more concisely as "When setting a value".

Thanks, I think that these are much clearer, especially when used
together. I have updated this in my local copy, so it will be picked up
when I send the next re-roll.

> >  --bool::
> > @@ -231,6 +235,9 @@ Valid `<type>`'s include:
> >         output it as the ANSI color escape sequence to the standard
> >         output.  The optional `default` parameter is used instead, if
> >         there is no color configured for `name`.
> > ++
> > +It is preferred to use `--type=color`, or `--type=color [--default=<default>]`
> > +instead of `--get-color`.
>
> Repetitious. More conscisely:
>
>     It is preferred to use `--type=color [--default=<value>]`
>     instead of `--get-color`.
>
> Or, even:
>
>     `--type=color [--default=<value>]` is preferred over `--get-color`.

Much clearer; I think that I prefer the second one. I have rolled this
into my local changes as above.


Thanks,
Taylor



[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