Re: [RFC] Colorization of log --graph

From: Allan Caffee <allan.caffee@gmail.com>
Date: Mar 19, 2009, 16:59 UTC
Message-ID: <b2e43f8f0903190959if539048r19e972899bd2132d@mail.gmail.com>
In-Reply-To: <alpine.DEB.1.00.0903181228420.10279@pacific.mpi-cbg.de>

Hello and thanks for the speedy reply!

On Wed, Mar 18, 2009 at 7:44 AM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 11 quoted lines

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

Show 6 quoted lines

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

Show 10 quoted lines

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