Re: [PATCHv3 2/3] cvsimport: fix the parsing of uppercase config options

From: Jeff King <peff@peff.net>
Date: Dec 1, 2010, 17:55 UTC
Message-ID: <20101201175517.GA7774@sigill.intra.peff.net>
In-Reply-To: <20101201163406.GD26120@burratino>

On Wed, Dec 01, 2010 at 10:34:06AM -0600, Jonathan Nieder wrote:

> (+cc: Jeff, config parsing wizard)

Ugh, that is not a title that I aspire to. :)

Show 7 quoted lines

> > Good idea, though I'd rather we avoid new convention for multi word
> > names separated with '-' (multi-word), but rather use camel case
> > (multiWord).
> 
> No disagreement there. :)
> 
> > Currently we have only unfortunate exception of `add.ignore-errors',

At some point there was talk of making this add.ignoreErrors in the docs, and just keeping the add.ignore-errors alias forever for backwards compatibility.

Show 5 quoted lines

> Maybe we can teach the config file parser to ignore dashes in addition
> to case (except in the names of [genus "species"] headings)?  That
> would be an incompatible change for third-party tools, though, so
> maybe "git config" would have to take an explicit --strip-dashes
> flag to do it.

But if you require --strip-dashes, then you get potentially differing behavior for the same set of options (i.e., one tool may accept "foobar" but the other requires "foo-bar", because the latter has not been updated to --strip-dashes).

Naively, such a patch would look like:

diff --git a/builtin/add.c b/builtin/add.c
index 22c6329..944c54f 100644
--- a/builtin/add.c
+++ b/builtin/add.c
@@ -331,7 +331,7 @@ static struct option builtin_add_options[] = {
 
 static int add_config(const char *var, const char *value, void *cb)
 {
-	if (!strcasecmp(var, "add.ignore-errors")) {
+	if (!strcasecmp(var, "add.ignoreerrors")) {
 		ignore_add_errors = git_config_bool(var, value);
 		return 0;
 	}
diff --git a/config.c b/config.c
index d8ce653..e5cb5f9 100644
--- a/config.c
+++ b/config.c
@@ -211,6 +211,8 @@ static int get_value(config_fn_t fn, void *data, char *name, unsigned int len)
 			break;
 		if (!iskeychar(c))
 			break;
+		if (c == '-')
+			continue;
 		name[len++] = tolower(c);
 		if (len >= MAXNAME)
 			return -1;

but there are a lot of corner cases. What happens with --get-regexp?
What happens with headings like '[foo-bar "baz"]'. When we use
"git-config" to edit, do we properly preserve dashes?

I started to think about all of these things, and then realized it
probably just isn't worth it. If we care about no-dashes, then let's
enforce no-dashes in new options. If we care about the existing
add.ignore-errors, then let's fix it but keep the old version there for
backwards compatibility. Those are easy to do, and then the problem just
goes away.

-Peff