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

From: Jeff King <[email-masked]>
Date: Tue, 26 Aug 2014 16:31:59 -0400

On Tue, Aug 26, 2014 at 01:20:53PM -0700, Junio C Hamano wrote:

> I think different people have different confusion criteria.
> To me, these two are very different operations:
> 
>     $ VAR=
>     $ unset VAR
> 
> I think it boils down to that I see that the distance between "unset
> vs set to empty" is far larger than the distance between "empty vs
> false".  You probably see these two distances the other way,
> i.e. "set to empty is almost like unset" and "empty is not a valid
> way to say false".
> 
> Due to this difference, the new test confused me and had me read it
> three times.

I agree that it is rather a subjective decision.

> So, I am not sure the patch in the message I am responding to, and I
> am not sure about that *v check in Steffen's patch, either.

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). 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.

I dunno. I can live with leaving it as-is. Certainly the existing
behavior is not what I expected, but it is not like it came up in the
real world (and I would not expect it to do so often). And it is
consistent with the config, which treats:

  [foo]
  bar =

as boolean false. That _also_ seems weird to me, but that is not
something I think we can easily change or outlaw at this point anyway.

-Peff