Re: git-config does not check validity of variable names

From: Jeff King <peff@peff.net>
Date: Jan 11, 2011, 05:59 UTC
Message-ID: <20110111055922.GD10094@sigill.intra.peff.net>
In-Reply-To: <20110108144644.GA11019@localhost.suse.cz>

On Sat, Jan 08, 2011 at 03:46:44PM +0100, Libor Pechacek wrote:

Show 7 quoted lines

> I've noticed that git-config accepts variable names in the form "a=b" for its
> "get" operation.  That means "git config a=b" does not write anything to its
> output and exists with status 1.
> 
> According to the man page only alphanumeric characters and - are allowed in
> variable names.  Would it make sense to spit out an error message when the user
> supplies an invalid variable name like the above?

Probably. The current behavior isn't all that terrible, in that it simply tries to look up the key, which of course doesn't exist (because it cannot syntactically), and does signal an error (with the exit code). So it is in some ways no worse than a typo like "git config color.dif.branch". And we probably don't want to start writing to stderr in such a case, as scripts assume they can call git config to find out whether the variable is defined without having to redirect stderr.

That being said, I can see how the lack of a message could be confusing for a user who mistakenly thinks "git config color.diff.branch=red" should work. So I think a patch to make that better would get a favorable response.

Note, though, that what you wrote above is not strictly true. The manpage says variable names and section names must be alphanumeric. But subsection names can contain any character except newline. So it is valid syntactically to do:

  git config color.diff=red.branch

where the subsection contains the "=". Obviously this example is nonsense, and in practice most such "a=b" forms will end up not being syntactically valid (because the = will be part of the variable name, not the subsection). But if you are going to write a patch, you need to make sure not to accidentally disallow:

  git config 'diff.my custom diff driver.command'

-Peff