From: Junio C Hamano <gitster@pobox.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: Alternative approach to the git config NULL value checking patches..
Date: Sun, 10 Feb 2008 14:29:01 -0800 [thread overview]
Message-ID: <7vbq6oe98y.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <alpine.LFD.1.00.0802101406560.2896@woody.linux-foundation.org> (Linus Torvalds's message of "Sun, 10 Feb 2008 14:08:58 -0800 (PST)")
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Sun, 10 Feb 2008, Junio C Hamano wrote:
> ...
>> will now need to be changed to:
>>
>> if (value == config_true)
>> Ah we have true;
>> else if (!*value)
>> Ok this is false;
>
> And that was done by my patch.
>
>> still need to be fixed to:
>>
>> if (value == config_true)
>> die("oops '%s' is not a bool", var);
>> else if (!strcmp(value, "somevalue")
>> Ok let's use somevalue;
>
> And this is different from checking against NULL exactly how?
Exactly. My answer to your question is: "It is not different
from checking against NULL at all."
The first one (i.e. you needed to do so in your patch) shows
that the codepath that is already doing the right thing needs to
be modified. If we do not introduce config_true, we do not even
have to. You need additional change to correctly functioning
codepaths that you should not have to do.
The second one shows that even if we introduce config_true, such
an already broken codepath needs to be fixed to check with
either NULL or config_true anyway. The need for fix the
codepath has not been reduced. Changing the rule does not help
for this class of codepath.
But as you seem to imply, it might make sense to equate
[some-random-section]
some-random-variable
to
[some-random-section]
some-random-variable = ""
for variables that cannot possibly have any meaningful "bool"
semantics. This third class of variables is a possible benefit
your patch brings in. The code can be lax for these variables.
However, it would make things inconsistent ("this variable is
bool and the above two forms mean completely opposite things,
while that variable is not bool and they mean the same thing").
I am just having a hard time convincing myself that this little
detail does not matter.
next prev parent reply other threads:[~2008-02-10 22:30 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-02-10 20:32 Alternative approach to the git config NULL value checking patches Linus Torvalds
2008-02-10 21:25 ` Junio C Hamano
2008-02-10 22:08 ` Linus Torvalds
2008-02-10 22:29 ` Junio C Hamano [this message]
2008-02-10 22:47 ` Junio C Hamano
2008-02-10 23:29 ` Pierre Habouzit
2008-02-10 23:50 ` Linus Torvalds
2008-02-10 23:36 ` Linus Torvalds
2008-02-10 23:41 ` Linus Torvalds
2008-02-11 0:40 ` Junio C Hamano
2008-02-11 7:22 ` Christian Couder
2008-02-11 8:12 ` Junio C Hamano
2008-02-11 17:27 ` Daniel Barkalow
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vbq6oe98y.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).