Re: [RFC] Colorization of log --graph

[Allan Caffee <allan.caffee@gmail.com>]

Hello and thanks for the speedy reply!

On Wed, Mar 18, 2009 at 7:44 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 18 Mar 2009, Allan Caffee wrote:
>
> > I know that _some_ people arn't particularly fond of colors, but I was
> > wondering how difficult it would be to colorize the edges on the --graph
> > drawn by the log command?  It can be a little tricky trying to follow
> > them with a relatively complex history.  I was thinking something like
> > gitk already does.
>
> That's a good idea!  (And it is mentioned as a TODO in graph.c...)

That's me, always thinking outside the box.  ;-)

> > Is anybody else interested in seeing this?
>
> Count me in.  Are you interested in implementing this?

I'll give it a go.  Been a while since I've done anything of substance
in pure C so it should be a nice refresher.  :)

> If so:
>
> - you need to #include "color.h" in graph.c
>
> - you need to insert a color identifier into struct column (there is an
>  XXX comment at the correct location)

By color identifier I assume you mean the ANSI escape sequence, right?
I didn't see a type for representing colors in color.{c,h} other than
the int it seems to use internally.

> - you need to find a way to determine colors for the branches

Okay, so if we were to make this similiar to how gitk works it would involve:
If the previous commit was a merge:
	for (i = 0; i < graph->num_columns; i++)
		graph->columns[i]->color = get_next_column_color();
else
	get_current_column_color();

I was thinking of storing the current color by adding a
default_column_color attribute to git_graph that serves as an index into
column_colors.  column_colors being the array of available colors.

> - you need to put the handling into the function
>  graph_output_pre_commit_line() in graph.c (and probably
>  graph_output_commit_char(), graph_output_post_merge_line(),
>  graph_output_collapsing_line(), graph_padding_line(), and
>  graph_output_padding_line(), too)
>
> - it would make sense IMHO to introduce a new function that takes a
>  pointer to an strbuf, a pointer to a struct column and a char (or maybe
>  a string) that adds the appropriately colorized char (or string) to the
>  strbuf

That makes sense.  Then we can just update the functions you mentioned
above to use this.

> - use the global variable diff_use_color to determine if the output should
>  be colorized at all

The function for adding a column to an strbuf would offer a convenient
place to put the condition.

> - probably you need to make an array of available colors or some such
>  (which might be good to put into color.[ch])

This would be the color_codes array I mentioned but it seems like it
might belong in graph.c.  There's something similiar in diff.c and it
seems like this is more related to graphing then to colors in general.
Although I do think it makes sense to #define some of the more common
ANSI codes there so that they don't have to be duplicated.  grep shows 6
occurrences of '\033[31m', the code for red foreground.

I'll begin working on a patch.  Comments/questions?

~Allan