Re: [PATCH 1/2] log.decorate: accept 0/1 bool values

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 17, 2010, 19:36 UTC
Message-ID: <7v4obfg2td.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20101117170045.GA4108@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> We cannot simply add 0/1 support to git_config_maybe_bool.
> That would confuse git_config_bool_or_int, which may want to
> distinguish the integer values "0" and "1" from bools.

... e.g. number one spelled as "1" do mean one not true in contexts like status.submodulesummary, where "true" means "unlimited" and "1" means "limit to one". It surely is necessary to avoid breakage there.

A caller that uses git_config_maybe_bool() expects to see 0/1 when the input looks like a boolean, and your patch looks like the right thing to do.

The repertoire of parsers that involve elements that are possibly boolean are now:

 - git_config_bool(): takes "0/false/no/..." or "$n/true/yes/..." (where
   any non-zero number $n is taken as true [*1*]), or more traditional

   [section]
       var

   without any equal sign, which means true.  Errors out if the input is
   not a boolean.

 - git_config_maybe_bool(): similar, and returns -1 (not a bool), 0
   (false) and 1 (true).  "0" is false, "1" is true.  But all other
   numbers are not boolean;

 - git_config_bool_or_int(): the caller can tell if the value was boolean.
   "0" and "1" are integers, and not boolean.  Errors out if the input is
   not bool nor int.

Perhaps we would want to add Documentation/technical/api-config.txt someday? Hint, hint....

[Footnote]

*1* I doubt anybody is insane enough to have an existing configuration file that spells "true" with "2" (or "100"), so it might be ok to tighten the rule after the fact a bit here to take only "1" as true from purist's point of view, but I do not think the upside outweighs possible breakage.