Re: [PATCH 4/5] config.c: add a "tristate" helper

From: Jeff King <peff@peff.net>
Date: Apr 10, 2021, 01:23 UTC
Message-ID: <YHD+BMo35krbH9rx@coredump.intra.peff.net>
In-Reply-To: <xmqqv98vm7lt.fsf@gitster.g>

On Fri, Apr 09, 2021 at 03:11:10PM -0700, Junio C Hamano wrote:

Show 13 quoted lines

> > I dunno. That would probably be hard to represent via "git config
> > --type". And some callers probably do care about "auto" versus "true".
> 
> It would work well for codepaths where computing the default value
> is cheap (or even possible).
> 
> I think the point of using "auto" is to delay the decision as late
> as possible.  E.g. in-core parsed config and attribute may still
> want to stay "auto", until we actually get our hands on the blob
> contents to see if it is binary, until we know how heavily loaded
> the system is, until we know isatty(1), etc.  Some are cheap to
> compute in advance, some are expensive and impossible until we meet
> the data.

Hmm, yeah, that's the "do care about auto versus true" thing.

Show 15 quoted lines

> So I still think the canonical use pattern for the "auto" thing is
> 
> 	is_frotz = git_parse_bool_or_auto(value);
> 
> 	... arbitrary number of things can happen here
> 	... the above may even be done in a git_config()
> 	... callback, and is_frotz may not even be used.
> 
> 	if (is_frotz == AUTO)
> 		is_frotz = auto_detect_frotz();
> 
> 	if (is_frotz)
> 		...; /* do the frotz thing */
> 	else
> 		...; /* do the non-frotz thing */

That makes sense. Usually we represent "undecided" in such a tristate with -1, so something that returned -1/0/1 would feel very natural to me (and probably wouldn't need symbolic constants even). But -1 is also error.

So a function like:

  int git_parse_tristate(const char *value, int *out);

which returned success/error via its return value, and put the value into "out" would feel pretty natural to me.

I dunno. I admit I don't care _that_ much, but somehow Ævar's series have a way of sniping me into responding anyway. :)

-Peff