Re: [PATCH v6 2/6] Add git_env_ulong() to parse environment variable

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 26, 2014, 21:54 UTC
Message-ID: <xmqq38cihq7w.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140826203158.GA30651@peff.net>

Jeff King <peff@peff.net> writes:

> If it is truly "some people prefer it one way and some the other", I am
> not sure if we should leave it as-is (that is preferring one way).

A worse position is to have git_env_bool() that says "empty is false" and add a new git_env_ulong() that says "empty is unset".

We should pick one or the other and use it for both.

As you pointed out in the later part of the message, an empty string is a valid way to spell "false" in the config subsystem since the beginning at 17712991 (Add ".git/config" file parser, 2005-10-10); consistency with it probably is sensible.

But perhaps my brain is rotten with too much Perl and Python X-<. I do not know where Linus picked up that, though ;-)

> The middle ground would be to die(). That does not seem super-friendly, but
> then we would also die with GIT_SMART_HTTP=foobar, so perhaps it is not
> unreasonable to just consider it a syntax error.

Hmm, I am not sure if dying is better. Unless we decide to make empty string no longer false everywhere and warn now and then later die as part of a 3.0 transition plan or something, that is.