Subject: Re: [PATCH/resend] add--interactive: allow diff colors without interactive colors

From: Matthias Kestenholz <[email-masked]>
Date: Sat, 05 Jan 2008 15:10:26 +0100

On Sat, 2008-01-05 at 03:11 -0800, Junio C Hamano wrote:
> Matthias Kestenholz <[email-masked]> writes:
> 
> > I managed to throw something together which works and passes all
> > the tests. Documentation included. :-)
> 
> Is it because we do not usually test colours and the tests run
> without terminals to make sure "color.* = auto" does not kick
> in?
> 

Probably, yes. I've to confess that I have not thought too much about
how color output could be tested. 

> > I would be happy for feedback and suggestions.
> 
>  * Shouldn't "color.git = true" with "color.diff = false" mean
>    "I want colour for everything by default but I do not want to
>    see coloured diff"?
> 

This works with the new patch below.

>  * git_foo_config() callback from git_config() returns 0 on
>    success; the API change needs to be documented to warn
>    others.
> 

I've changed the config reader. It behaves like the others now.

> I haven't studied your patch very deeply so I may have misread
> what you tried to do, regarding the first point, though.

Still missing are the updates for git add -i and git svn.

I have added a new function git_color_config() which should be called
after git_config() has finished its work. The function needs to run when
all config reading has been done, because otherwise the color setup
would be sensitive to the order in which the variables are placed in the
config file, which I'd rather avoid.

I've also unified the colorbool variables, because otherwise the
individual colorbool variables would need to be exported into other C
files, and I don't see the point of multiple colorbools anyway.

The patch got somewhat big, therefore I've splitted it up into several
pieces:

[PATCH 1/4] Add infrastructure for a single color config variable
[PATCH 2/4] git branch: Use color configuration infrastructure
[PATCH 3/4] status and commit: Use color configuration infrastructure
[PATCH 4/4] diff and log: Use color configuration infrastructure