From: Junio C Hamano <gitster@pobox.com>
To: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
Cc: git@vger.kernel.org, "Lin Sun" <lin.sun@zoom.us>,
"Đoàn Trần Công Danh" <congdanhqx@gmail.com>,
"David Aguilar" <davvid@gmail.com>
Subject: Re: [PATCH 1/5] config.c: add a comment about why value=NULL is true
Date: Thu, 08 Apr 2021 11:10:50 -0700 [thread overview]
Message-ID: <xmqqmtu8tzo5.fsf@gitster.g> (raw)
In-Reply-To: <patch-1.6-bd70181ffb-20210408T133125Z-avarab@gmail.com> ("Ævar Arnfjörð Bjarmason"'s message of "Thu, 8 Apr 2021 15:34:25 +0200")
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> Subject: Re: [PATCH 1/5] config.c: add a comment about why value=NULL is true
A few unproductive nitpicks.
The question "why does '[core] ignorecase' mean core.ignorecase is
true?" has only a single answer, which is "because that is how we
designed (loosely imitating .ini format) and implemented."
> It's not very intuitive that git_parse_maybe_bool_text() would
> consider NULL to be a true value. Add a small comment about it.
Do you mean
if (!value)
return 1;
is unintuitive? Or do you mean
[core]
ignorecase
being a valid way to say "the filesystem ignores case differences"?
The answer to the question is crucial to justify the title. Only
when "the config syntax is perfectly understandable, the way the
logic to parse that syntax is expressed in C is not necessarily
intuitive" is true, the comment makes sense, I would think.
> See a789ca70e7 (config: teach "git -c" to recognize an empty string,
> 2014-08-04) for the behavior of "git -c", but we'll end up here in
> both the config file parsing and command-line parsing cases.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> config.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/config.c b/config.c
> index 6428393a41..fc28dbd97c 100644
> --- a/config.c
> +++ b/config.c
> @@ -1229,6 +1229,10 @@ ssize_t git_config_ssize_t(const char *name, const char *value)
> static int git_parse_maybe_bool_text(const char *value)
> {
> if (!value)
> + /*
> + * "[foo]\nbar\n" and "-c foo.bar" on the command-line
> + * are true.
> + */
This is a "we do not explain why '[foo] bar' is true, but to be
prepared to parse it, we check if value is NULL" comment. And
I think it is a good one, except that I do not see the point of
writing "\n" at all. Replacing them with spaces would make it far
easier to read.
/*
* "[foo] bar" in the configuration file, and
* "git -c foo.bar" on the command-line, mean
* that the variable foo.bar is true. A NULL is
* passed as 'value' in these cases.
*/
> return 1;
> if (!*value)
> return 0;
next prev parent reply other threads:[~2021-04-08 18:10 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-04-08 13:34 [PATCH 0/5] config: support --type=bool-or-auto for "tristate" parsing Ævar Arnfjörð Bjarmason
2021-04-08 13:34 ` [PATCH 1/5] config.c: add a comment about why value=NULL is true Ævar Arnfjörð Bjarmason
2021-04-08 18:10 ` Junio C Hamano [this message]
2021-04-08 13:34 ` [PATCH 2/5] config tests: test for --bool-or-str Ævar Arnfjörð Bjarmason
2021-04-08 18:21 ` Junio C Hamano
2021-04-08 23:11 ` Ævar Arnfjörð Bjarmason
2021-04-08 13:34 ` [PATCH 3/5] git-config: document --bool-or-str and --type=bool-or-str Ævar Arnfjörð Bjarmason
2021-04-08 18:22 ` Junio C Hamano
2021-04-08 13:34 ` [PATCH 4/5] config.c: add a "tristate" helper Ævar Arnfjörð Bjarmason
2021-04-08 18:33 ` Junio C Hamano
2021-04-08 23:23 ` Ævar Arnfjörð Bjarmason
2021-04-08 23:51 ` Junio C Hamano
2021-04-09 1:33 ` Ævar Arnfjörð Bjarmason
2021-04-09 12:53 ` Junio C Hamano
2021-04-08 23:54 ` Junio C Hamano
2021-04-09 20:05 ` Jeff King
2021-04-09 22:11 ` Junio C Hamano
2021-04-10 1:23 ` Jeff King
2021-04-10 1:43 ` Junio C Hamano
2021-04-08 13:34 ` [PATCH 5/5] config: add --type=bool-or-auto switch Ævar Arnfjörð Bjarmason
2021-04-08 18:36 ` Junio C Hamano
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqmtu8tzo5.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=congdanhqx@gmail.com \
--cc=davvid@gmail.com \
--cc=git@vger.kernel.org \
--cc=lin.sun@zoom.us \
/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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).