Re: [PATCH] graph API: Added logic for colored edges
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 8, 2009, 07:59 UTC
- Message-ID
- <7vd4bnpodt.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20090407185724.GA9996@linux.vnet>
Allan Caffee <allan.caffee@gmail.com> writes:
Show 9 quoted lines
> @@ -72,11 +69,21 @@ 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.
> */
> + const char *color;You already use short for git_graph.default_column_color and I suspect in the longer term you want to make this one an index into column_colors[] array the same way. We may someday decide to support non-ANSI color scheme using ncurses or something, and at that point the "hardware color" constants like GIT_COLOR_RED and friends may change from strings to small integers we use to call our (yet to be written) curses interface layer with.
Show 5 quoted lines
> @@ -312,6 +343,33 @@ static struct commit_list *first_interesting_parent(struct git_graph *graph) > return next_interesting_parent(graph, parents); > } > > +static const char* graph_get_current_column_color(const struct git_graph* graph)
Style. Asterisk comes next to identifiers, not types (the parameter to graph_increment_column_color has the same issue).
static const char *graph_...color(const struct git_graph *graph)
Show 6 quoted lines
> @@ -596,7 +661,7 @@ static void graph_output_padding_line(struct git_graph *graph,
> * Output a padding row, that leaves all branch lines unchanged
> */
> for (i = 0; i < graph->num_new_columns; i++) {
> - strbuf_addstr(sb, "| ");
> + strbuf_write_column(sb, &graph->new_columns[i], "| ");Hmmm, this forbids us to use reverse color in the color palette because that would highlight the trailing whitespace. Is that something we care about, or a reversed "|", "/" and "\" are already too ugly that we won't want to support them?
Show 10 quoted lines
> @@ -648,8 +713,11 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
> for (i = 0; i < graph->num_columns; i++) {
> struct column *col = &graph->columns[i];
> if (col->commit == graph->commit) {
> + struct strbuf tmp = STRBUF_INIT;
> seen_this = 1;
> - strbuf_addf(sb, "| %*s", graph->expansion_row, "");
> + strbuf_addf(&tmp, "| %*s", graph->expansion_row, "");
> + strbuf_write_column(sb, col, tmp.buf);
> + strbuf_release(&tmp);Same here.
Show 17 quoted lines
> @@ -662,13 +730,13 @@ static void graph_output_pre_commit_line(struct git_graph *graph,
> */
> if (graph->prev_state == GRAPH_POST_MERGE &&
> graph->prev_commit_index < i)
> - strbuf_addstr(sb, "\\ ");
> + strbuf_write_column(sb, col, "\\ ");
> else
> - strbuf_addstr(sb, "| ");
> + strbuf_write_column(sb, col, "| ");
> } else if (seen_this && (graph->expansion_row > 0)) {
> - strbuf_addstr(sb, "\\ ");
> + strbuf_write_column(sb, col, "\\ ");
> } else {
> - strbuf_addstr(sb, "| ");
> + strbuf_write_column(sb, col, "| ");
> }
> }Likewise.
Show 23 quoted lines
> @@ -744,14 +813,25 @@ static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
> if (graph->num_parents < 3)
> strbuf_addch(sb, ' ');
> else {
> + /*
> + * 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 num_dashes =
> - ((graph->num_parents - 2) * 2) - 1;
> + ((graph->num_parents - dashless_commits) * 2) - 1;
> for (j = 0; j < num_dashes; j++)
> - strbuf_addch(sb, '-');
> - strbuf_addstr(sb, ". ");
> + strbuf_write_column(sb,
> + &graph->new_columns[(j / 2) + dashless_commits],
> + "-");
> + strbuf_write_column(sb,
> + &graph->new_columns[(j / 2) + dashless_commits],
> + ". ");The nesting seems to be becoming too deep and the body of the for loop is getting too long. Time to make it a helper function that handles only one column, perhaps?