Hi,
On Thu, 19 Mar 2009, Allan Caffee wrote:
> Hello and thanks for the speedy reply!
Heh, Git is known for raw speed ;-)
> On Wed, Mar 18, 2009 at 7:44 AM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
>
> > On Wed, 18 Mar 2009, Allan Caffee wrote:
> >
> > > 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. :)
Great!
> > 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.
I'd actually add an enum color_names, or something like that.
> > - 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.
Yep, I agree. That index could be of type "enum color_names" if you
introduce the latter...
> > - 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.
Right.
> > - 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.
Yes!
> > - 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'd actually like to see it in color.[ch], so that other code paths can
use it, too.
I'd start like this:
enum color_name {
COLOR_RESET,
COLOR_RED,
COLOR_GREEN,
COLOR_YELLOW,
COLOR_BLUE,
COLOR_MAGENTA,
COLOR_CYAN,
COLOR_WHITE
};
Maybe the best thing would then be to add a function
void strbuf_add_color(struct strbuf *buf, enum color_name name) {
if (name == COLOR_RESET)
strbuf_addf(buf, "\033[m");
else
strbuf_addf(buf, "\033[%dm", 31 + name - COLOR_RED);
}
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
