Re: [PATCH v3] graph API: Added logic for colored edges

[Junio C Hamano <gitster@pobox.com>]

Allan Caffee <allan.caffee@gmail.com> 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.

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