list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <>
To: Junio C Hamano <>
Cc:, "Lin Sun" <>,
	"Đoàn Trần Công Danh" <>,
	"David Aguilar" <>, "Jeff King" <>
Subject: Re: [PATCH 4/5] config.c: add a "tristate" helper
Date: Fri, 09 Apr 2021 03:33:16 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <xmqqtuogpc6f.fsf@gitster.g>

On Fri, Apr 09 2021, Junio C Hamano wrote:

> Ævar Arnfjörð Bjarmason <> writes:
>> I'd prefer to just make these "enum", which means we'll have the aid of
>> the compiler in checking all the callsites, as in the patch-on-top
>> (which I can squash appropriately, need to update the doc comments
>> though) at the end of this E-Mail.
> I think enum is oversold by some people (not me).  C Compilers won't
> do much when you use them interchangeably with int, simply because
> they are designed to be used that way, no?

They are just ints, not a seperate type like in C++.

> If existing code used 0 as false and 1 as true, and it learns an
> "auto" value with a new definition,
>     #define TRISTATE_AUTO 2
> without TRISTATE_{TRUE,FALSE} defined to 0 and 1, that would be a
> good place to stop.  I'd be quite happy with that.

The benefit in this case is to human readers of the code who know
they're being helped by the enum checking in "case" arms.

So you can immediately look at code like:

    enum foo x = git_config_foo();
    switch (x)

And know without having to jump to the definition of "git_config_foo()"
that all the return values are being handled.

Just to clarify, do you have a dislike of the sort of code in for all
uses in the codebase, or in this case because the diff of adapting
existing code is larger as a result?

I think if we're not going to use enum/switch for this and similar
things in config.c in the future it makes sense to pass a pointer to a
"is_auto" parameter to these new tristate() functions, similar to
e.g. the existing git_config_bool_or_int().

  reply	other threads:[~2021-04-09  1:33 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
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 [this message]
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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 4/5] config.c: add a "tristate" helper' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

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