All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: Stefan Beller <sbeller@google.com>, Git List <git@vger.kernel.org>
Subject: Re: [PATCH] color.h: document and modernize header
Date: Thu, 8 Feb 2018 17:28:51 -0500	[thread overview]
Message-ID: <20180208222851.GA8850@sigill.intra.peff.net> (raw)
In-Reply-To: <CAPig+cTZuf5rMNN6aSztX-jKrJkuRpVcJj+xmrSsO6ngrHhU6Q@mail.gmail.com>

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

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

  reply	other threads:[~2018-02-08 22:28 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-08 20:15 [PATCH] color.h: document and modernize header Stefan Beller
2018-02-08 20:43 ` Jeff King
2018-02-08 21:04   ` Stefan Beller
2018-02-08 21:38     ` [PATCH] CodingGuidelines: mention "static" and "extern" Jeff King
2018-02-08 21:43       ` Stefan Beller
2018-02-08 23:14       ` Eric Sunshine
2018-02-09 19:07         ` Jonathan Tan
2018-02-09 19:33           ` Jeff King
2018-02-08 22:26   ` [PATCH] color.h: document and modernize header Eric Sunshine
2018-02-08 22:28     ` Jeff King [this message]
2018-02-12 20:19       ` Stefan Beller
2018-02-12 22:14         ` Eric Sunshine
2018-02-13  1:41           ` Stefan Beller
2018-02-13  3:55             ` Eric Sunshine
2018-02-14  7:23               ` Eric Sunshine
2018-02-14 17:58                 ` Stefan Beller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180208222851.GA8850@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.