Re: [PATCH] completion: add missing configuration variables

On Wed, Dec 15, 2010 at 07:46:53AM +0100, Martin von Zweigbergk wrote:

The color.grep.external option has been deleted. Should it be deleted
from here or do we want to help users run e.g.
'git config --unset color.grep.external'? Same goes for
add.ignore-errors.

IMHO, they should go away. People who have them can figure out how to
delete them, but it is more important not to advertise them to people
who are adding variables.

As an aside, I would think "--unset" should actually choose from the set
of configured variables for completion (i.e., "git config --list | cut
-d= -f1"). But that would obviously be a separate patch.

I didn't find any references to 'diff.renameLimit.' even in 98171a0
(bash completion: Sync config variables with their man pages,
2008-12-15) in which it was introduced in the completions script. I
hope it was safe to remove it.

Yeah, I don't think it has ever existed.

Some variables are documented with camelCase but read in all
lowercase in the code. Not worth updating the code just for that, is
it?

All variables are case-insensitive. The config parser down-cases them,
so all code should treat tham as all-lowercase. However, we tend to
document them as camelCase for readability.

The completion code should match case-insensitively, too. It doesn't
seem to now, but I suspect it is not a problem in practice because the
first camelCase word is often enough to get a match, and is lowercase
itself.

I hope none of the added variables are deprecated. After having a
quick look in git-config(1), I think they should not be.

All looked OK from my quick glance.

One note:

 		color.diff
 		color.diff.commit
 		color.diff.frag
+		color.diff.func
 		color.diff.meta
 		color.diff.new
 		color.diff.old
 		color.diff.plain
 		color.diff.whitespace

We have color.diff.branch coming soon (I think it is in 'next' now).

-Peff