Re: [PATCH v6 2/6] Add git_env_ulong() to parse environment variable
flat view
From: Jeff King <hidden>
Date: 2016-06-15 23:02:21
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