Re: [PATCH] completion: add missing configuration variables

[Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>]

On Wed, 15 Dec 2010, Jeff King wrote:

> 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.

Sounds good to me. If no one disagrees, I'll send an updated patch in
a day or two, removing both color.grep.external and add.ignore-errors.

> 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.

Good point. I'll put it on my todo.

> > 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.

That I knew...

> The config parser down-cases them,
> so all code should treat tham as all-lowercase.

... but not that. Thanks. I could/should have figured that out if I
had noticed that *all* of them were lowercase in the code :-P.

> 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).

Strictly speaking, that note is for Junio to think of when he merges,
right? But adding it early is pretty harmless and if that relieves him
of some work, I would be happy to add it in the next submission of
this patch. Is that better?

Thinking a bit more, maybe what you are suggesting is that I base the
next revision of this patch on the branch that adds that variable?


/Martin