Re: [PATCH 1/2] handle color.ui at a central place

From: Markus Heidelberg <markus.heidelberg@web.de>
Date: Jan 24, 2009, 11:28 UTC
Message-ID: <200901241228.33690.markus.heidelberg@web.de>
In-Reply-To: <7vmydi4kiz.fsf@gitster.siamese.dyndns.org>

Junio C Hamano, 23.01.2009:

Show 21 quoted lines

> Markus Heidelberg <markus.heidelberg@web.de> writes:
> 
> > Jeff King, 22.01.2009:
> > ...
> >> > ...
> >> > That would probably be better.
> >> 
> >> Do you want to work on it, or should it go into the "yeah, right, one
> >> day" section of my todo list?
> >
> > Yes, feel free to enlarge your todo list :)
> > There are some other things that I want to work on before, so better
> > don't count on me for this. But maybe I'll come up to it, before your
> > todo list pointer reaches this item, who knows.
> 
> Whatever.
> 
> I merged and pushed out these two patches but they seem to break
> format-patch big time if you have ui.color set to auto.
> 
> I will be reverting them out of 'next'.  Grumble.

Damn, sorry for this.

I looked at the code and found this in git_format_config():

	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
		return 0;

Which of course didn't handle color.ui, but that wasn't necessary before the central color.ui handling from my patch.

So with the following diff it works:

- if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) { + if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff") + || !strcmp(var, "color.ui")) {

Compared to f3aafa4 (Disable color detection during format-patch, 2006-07-09) and a159ca0 (Allow subcommand.color and color.subcommand color configuration, 2006-12-13), which introduced !strcmp(var, "diff.color") resp. !strcmp(var, "color.diff") at this place.

Or with this, which however would be a similar thing to what I tried to remove from the code.

        git_config(git_format_config, NULL);
+       diff_use_color_default = 0;

format-patch is perhaps the only place where the commit has broken things, because I didn't find other places, where color config options were set, but not the corresponding variables. So it seems as if only format-patch needed code like this to turn off the colors.

Markus