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

[Jeff King <peff@peff.net>]

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

> 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