From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 22 Apr 2009 21:16:25 GMT
Subject: Re: [PATCH] show-branch: color the commit status signs
Message-ID: <7vskk01j9y.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <200904222245.24034.markus.heidelberg@web.de>

Markus Heidelberg <markus.heidelberg@web.de> writes:

> Stephen Boyd, 22.04.2009:
>> On Tue, Apr 21, 2009, Markus Heidelberg <markus.heidelberg@web.de> wrote:
>> > +--color::
>> > +       Color the status sign of the commits. Each branch uses a single color.
>> 
>> I don't know what the status sign of a commit is without reading your commit
>> message. Maybe reword this to be:
>> 
>> Color the status sign ('*' '!' '+' '-') of each commit corresponding
>> to the branch
>> it's in.
>
> I had some problems with finding the right words for the description and
> still wasn't satisfied with it. I think your suggestion makes it easier
> and will include it.
>
>> > +static char column_colors[][COLOR_MAXLEN] = {
>> > +       GIT_COLOR_RED,
>> > +       GIT_COLOR_GREEN,
>> > +       GIT_COLOR_YELLOW,
>> > +       GIT_COLOR_BLUE,
>> > +       GIT_COLOR_MAGENTA,
>> > +       GIT_COLOR_CYAN,
>> > +};
>> > +
>> > +#define COLUMN_COLORS_MAX (ARRAY_SIZE(column_colors))
>> 
>> This looks like a copy/paste (with a small edit) from graph.c so maybe you
>> want to refactor the original code from graph.c to color.h?
>
> How exactly do you think it can be refactored and then combined?

These tables belong to each command.  Your show-branches do not
necessarily want to get different set or number of colors when graph
output updates its color palette to have richer set of colors.  I do not
see any need for "refactoring" here.