From: Taylor Blau <me@ttaylorr.com>
Date: Thu, 05 Apr 2018 02:48:45 GMT
Subject: Re: [PATCH v3 3/3] builtin/config: introduce `color` type specifier
Message-ID: <20180405024845.GD31883@syl.local>
In-Reply-To: <xmqqtvsx3060.fsf@gitster-ct.c.googlers.com>

On Fri, Mar 30, 2018 at 11:09:43AM -0700, Junio C Hamano wrote:
> Taylor Blau <me@ttaylorr.com> writes:
>
> > @@ -184,6 +183,7 @@ Valid `[type]`'s include:
> >  --bool-or-int::
> >  --path::
> >  --expiry-date::
> > +--color::
> >    Historical options for selecting a type specifier. Prefer instead `--type`,
> >    (see: above).
> >
> > @@ -223,6 +223,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`.
>
> Wasn't the whole point of the preliminary --type=<type> patch to
> avoid having to add thse two hunks?

For the first hunk, yes, but not for the second. The series that adds
"--type=<type>" was meant to make it possible to say "parse this as a
color" without squatting on the "--color" flag.

So, including "--color" in the list of historical options is a mistake.
But, using "--type=color --default=..." over "--get-color" is the
desired intention of this series.


Thanks,
Taylor