Re: [PATCH v8 03/12] Move lower case functions into wrapper.c

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 27, 2014, 22:47 UTC
Message-ID: <xmqqha6jcl96.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140327223406.GA32434@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

> All bool config values allow "tRuE".

I was expecting somebody will bring it up, but think about it. Bool is a very special case. Even among CS folks, depending on your background, true may be True may be TRUE may be 1.

Conflating it with some random enum does not make a good argument.

> Ones that take "auto" often use
> strcasecmp (e.g., diff.*.binary). "blame.date" and "help.format" choose
> from a fixed set of tokens, but use strcmp.

I would say that the latter is the right thing to do.

> In general I do not see any reason _not_ to use strcasecmp for config
> values that are matching a fixed set. It's friendlier to the user,...

Actually, I think it ends up being hostile to the users to accept random cases without a good reason. If you see two trailer elements whose where are specified as "after" and "AFTER" in somebody's configuration file, wouldn't that give a wrong impression that a new line with the latter somehow has a stronger desire to come later than the former?

If you consistently take only the fixed strings, you do not have to worry about many people writing the same things in different ways, confusing each other.

I would however fully agree with you that using strcasecmp() would be the cleanest when reading and maintaining the code **IF** we want to accept values in random case, but I do not agree that accepting random cases is a good thing, so...