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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 8, 2021, 23:54 UTC
Message-ID: <xmqqsg40pc1l.fsf@gitster.g>
In-Reply-To: <875z0wicmp.fsf@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 22 quoted lines

>>> +int git_parse_maybe_tristate(const char *value)
>>> +{
>>> +	int v = git_parse_maybe_bool(value);
>>> +	if (v < 0 && !strcasecmp(value, "auto"))
>>> +		return 2;
>>> +	return v;
>>> +}
>>
>> This is not parse_maybe_bool_text(), so "1" and "-1" written in the
>> configuration file are "true", "0" is "false", like the "bool" case.
>>
>> I wonder if written without an unnecessary extra variable, i.e.
>>
>> 	if (value && !strcasecmp(value, "auto"))
>> 		return 2;
>> 	return git_parse_maybe_bool(value);
>>
>> is easier to follow, though, as it is quite clear that it is mostly
>> the same as maybe_bool and the only difference is when "auto" is
>> given.
>
> I guess it could be either way around,...

Having seen another example in the current code recently,

 static int parse_tristate(int *b, const char *k, const char *v)
 {
-	if (v && !strcasecmp(v, "auto"))
-		*b = -1;
-	else
-		*b = git_config_bool(k, v);

I upgrade my earlier "I wonder" to "I do think that". Let's swap the order so that it is clear that we are special-casing "auto".