On Sun, 12 Apr 2009, Junio C Hamano wrote:
> Allan Caffee <allan.caffee@xxxxxxxxx> writes:
>
> > diff --git a/graph.c b/graph.c
> > index 162a516..beb622a 100644
> > --- a/graph.c
> > +++ b/graph.c
> > @@ -1,5 +1,6 @@
> > #include "cache.h"
> > #include "commit.h"
> > +#include "color.h"
> > #include "graph.h"
> > #include "diff.h"
> > #include "revision.h"
> > @@ -72,11 +69,14 @@ struct column {
> > */
> > struct commit *commit;
> > /*
> > - * XXX: Once we add support for colors, struct column could also
> > - * contain the color of its branch line.
> > + * The color to (optionally) print this column in. This is an
> > + * index into column_colors.
> > */
> > + unsigned short color;
> > };
> >
> > +const unsigned short GIT_NOT_A_COLOR = -1;
>
> That (-1) is an unusual value for an *unsigned* short variable.
Perhaps you would prefer USHRT_MAX? I noticed that none of the existing
code #includes limits.h. Is it safe to assume this header is present?
> > @@ -714,10 +790,30 @@ static void graph_output_commit_char(struct git_graph *graph, struct strbuf *sb)
> > strbuf_addch(sb, '*');
> > }
> >
> > +inline void graph_draw_octopus_merge(const struct git_graph *graph,
> > + struct strbuf *sb)
> > +{
> > + /*
> > + * Here dashless_commits represents the number of parents
> > + * which don't need to have dashes (because their edges fit
> > + * neatly under the commit).
> > + */
> > + const int dashless_commits = 2;
> > + int col_num, i;
> > + int num_dashes =
> > + ((graph->num_parents - dashless_commits) * 2) - 1;
> > + for (i = 0; i < num_dashes; i++) {
> > + col_num = (i / 2) + dashless_commits;
> > + strbuf_write_column(sb, &graph->new_columns[col_num], '-');
>
> graph.c: In function 'graph_draw_octopus_merge':
> graph.c:807: error: 'strbuf_write_column' is static but used in inline function 'graph_draw_octopus_merge' which is not static
> graph.c:810: error: 'strbuf_write_column' is static but used in inline function 'graph_draw_octopus_merge' which is not static
> make: *** [graph.o] Error 1
>
> In general, I'd prefer people not to say "inline" unless (1) they know
> what they are doing, and (2) the code is really performance critical.
>
> At least I do not think the colored commit graph is performance critical,
> especially a function that only deals with octopus merges.
Okay. I'll remove the inline specifier from all the functions I added.
--
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