Re: [PATCH] core.abbrev <off|false|no> disables abbreviations

[Junio C Hamano <gitster@pobox.com>]

Derrick Stolee <stolee@gmail.com> writes:

>> +		else if (!strcasecmp(value, "false") ||
>> +			 !strcasecmp(value, "no") ||
>> +			 !strcasecmp(value, "off"))
>> +			default_abbrev = the_hash_algo->hexsz;
>
> I'm not sure we need three synonyms for "no-abbrev" here.

I do not particularly mind, but if we imitate the variety of various
boolean false, I'd prefer to see the code to parse them shared to
avoid them drifting apart over time.

> "false" would be natural, except I think in a few places
> the config value "0" is also interpreted as "false", but
> as seen below a value of "0" snaps up to the minimum
> allowed abbreviation.

I was in the vicinity of this code recently for reviewing another
topic, but IIRC, 0 came from the UI level does get rounded up to the
minimum accepted and never reach "default_abbrev", but if you manage
to place 0 or -1 in default_abbrev here (e.g. with additional code,
like the above part with the right hand side of the assignment
updated), I think the value will propagate throughout the codepath
and causes the downstream code to do the right thing.  0 will give
you no-abbreviation (i.e. full length depending on the length of the
hash) and -1 will give you the "scale as appropriate for the size of
the object store".

I have mild preference for using 0 over hardcoded single "full
length" here.  Even though we currently do not plan to allow
multiple hashes in use simultaneously in a single invocation of Git,
if that ever happens, we will regret hardcoding the_hash_algo->hexsz
on the right hand side of the assignment here, like this patch does.

Telling the downstream code in the control flow that we want no
truncation by using 0 would keep both 40-hexdigit and 64-hexdigit
hashes to their original length (as opposed to telling it to
truncate at 40 or 64 by using the_hash_algo->hexsz).

Thanks.