Re: [PATCH] color.h: document and modernize header

From: Jeff King <peff@peff.net>
Date: Feb 8, 2018, 22:28 UTC
Message-ID: <20180208222851.GA8850@sigill.intra.peff.net>
In-Reply-To: <CAPig+cTZuf5rMNN6aSztX-jKrJkuRpVcJj+xmrSsO6ngrHhU6Q@mail.gmail.com>

On Thu, Feb 08, 2018 at 05:26:33PM -0500, Eric Sunshine wrote:

Show 13 quoted lines

> > I think the point is that "var" is a quad-state variable (yes, no, auto,
> > or "unknown") and we are converting to a boolean. This would probably be
> > a lot more clear if GIT_COLOR_* were all enum values and not #defines,
> > and this function took the matching enum type.
> >
> > So I think that's what you were trying to name with "constants as
> > returned by...", but it definitely took me some thinking to parse it.
> 
> Rather than talking about plural "constants" (which makes it more
> confusing), it would likely be helpful for it to say (explicitly) that
> the caller passes in the result of git_config_colorbool() as 'var'.
> 
> Or something like that.

That's not quite it, though. "var" is really the current program's idea of whether color has been requested. So it's git_config_colorbool(), as modified by things like "--color".

I think the best explanation is that it resolves an "enum color_bool" into a single 0/1 boolean. Except that "enum color_bool" doesn't exist, so we have no name for it.

-Peff