@matheusbsbs / matheus-bagliano / commits / bbb13e8188

graph: fix coloring of octopus dashes

In 04005834ed ("log: fix coloring of certain octopus merge shapes", 2018-09-01) there is a fix for the coloring of dashes following an octopus merge. It makes a distinction between the case where all parents introduce a new column, versus the case where the first parent collapses into an existing column: | *-. | *-. | |\ \ | |\ \ | | | | |/ / / The latter case means that the columns for the merge parents begin one place to the left in the `new_columns` array compared to the former case. However, the implementation only works if the commit's parents are kept in order as they map onto the visual columns, as we get the colors by iterating over `new_columns` as we print the dashes. In general, the commit's parents can arbitrarily merge with existing columns, and change their ordering in the process. For example, in the following diagram, the number of each column indicates which commit parent appears in each column. | | *---. | | |\ \ \ | | |/ / / | |/| | / | |_|_|/ |/| | | 3 1 0 2 If the columns are colored (red, green, yellow, blue), then the dashes will currently be colored yellow and blue, whereas they should be blue and red. To fix this, we need to look up each column in the `mapping` array, which before the `GRAPH_COLLAPSING` state indicates which logical column is displayed in each visual column. This implementation is simpler as it doesn't have any edge cases, and it also handles how left-skewed first parents are now displayed: | *-. |/|\ \ | | | | 0 1 2 3 The color of the first dashes is always the color found in `mapping` two columns to the right of the commit symbol. Because commits are displayed after all edges have been collapsed together and the visual columns match the logical ones, we can find the visual offset of the commit symbol using `commit_index`. Signed-off-by: James Coglan <jcoglan@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

James Coglan committed Oct 15, 2019 at 23:47 UTC bbb13e8188ee37dd3e2318752342622266659620

2 files changed +42 -39

graph.c

+37 -34

@@ -684,6 +684,11 @@ static void graph_update_columns(struct git_graph *graph)

684 graph->mapping_size--;

685 }

686

687 +static int graph_num_dashed_parents(struct git_graph *graph)

688 +{

689 + return graph->num_parents + graph->merge_layout - 3;

690 +}

691 +

692 static int graph_num_expansion_rows(struct git_graph *graph)

693 {

694 /*

@@ -706,7 +711,7 @@ static int graph_num_expansion_rows(struct git_graph *graph)

711 * | * \

712 * |/|\ \

713 */

709 - return (graph->num_parents + graph->merge_layout - 3) * 2;

714 + return graph_num_dashed_parents(graph) * 2;

715 }

716

717 static int graph_needs_pre_commit_line(struct git_graph *graph)

@@ -934,47 +939,45 @@ static void graph_output_commit_char(struct git_graph *graph, struct graph_line

939 static void graph_draw_octopus_merge(struct git_graph *graph, struct graph_line *line)

940 {

941 /*

937 - * Here dashless_parents represents the number of parents which don't

938 - * need to have dashes (the edges labeled "0" and "1"). And

939 - * dashful_parents are the remaining ones.

942 + * The parents of a merge commit can be arbitrarily reordered as they

943 + * are mapped onto display columns, for example this is a valid merge:

944 *

941 - * | *---.

942 - * | |\ \ \

943 - * | | | | |

944 - * x 0 1 2 3

945 + * | | *---.

946 + * | | |\ \ \

947 + * | | |/ / /

948 + * | |/| | /

949 + * | |_|_|/

950 + * |/| | |

951 + * 3 1 0 2

952 *

946 - */

947 - const int dashless_parents = 3 - graph->merge_layout;

948 - int dashful_parents = graph->num_parents - dashless_parents;

949 -

950 - /*

951 - * Usually, we add one new column for each parent (like the diagram

952 - * above) but sometimes the first parent goes into an existing column,

953 - * like this:

953 + * The numbers denote which parent of the merge each visual column

954 + * corresponds to; we can't assume that the parents will initially

955 + * display in the order given by new_columns.

956 *

955 - * | *-.

956 - * |/|\ \

957 - * | | | |

958 - * x 0 1 2

957 + * To find the right color for each dash, we need to consult the

958 + * mapping array, starting from the column 2 places to the right of the

959 + * merge commit, and use that to find out which logical column each

960 + * edge will collapse to.

961 *

960 - * In which case the number of parents will be one greater than the

961 - * number of added columns.

962 + * Commits are rendered once all edges have collapsed to their correct

963 + * logcial column, so commit_index gives us the right visual offset for

964 + * the merge commit.

965 */

963 - int added_cols = (graph->num_new_columns - graph->num_columns);

964 - int parent_in_old_cols = graph->num_parents - added_cols;

966

966 - /*

967 - * In both cases, commit_index corresponds to the edge labeled "0".

968 - */

969 - int first_col = graph->commit_index + dashless_parents

970 - - parent_in_old_cols;

967 + int i, j;

968 + struct column *col;

969

972 - int i;

973 - for (i = 0; i < dashful_parents; i++) {

974 - graph_line_write_column(line, &graph->new_columns[i+first_col], '-');

975 - graph_line_write_column(line, &graph->new_columns[i+first_col],

976 - i == dashful_parents-1 ? '.' : '-');

970 + int dashed_parents = graph_num_dashed_parents(graph);

971 +

972 + for (i = 0; i < dashed_parents; i++) {

973 + j = graph->mapping[(graph->commit_index + i + 2) * 2];

974 + col = &graph->new_columns[j];

975 +

976 + graph_line_write_column(line, col, '-');

977 + graph_line_write_column(line, col, (i == dashed_parents - 1) ? '.' : '-');

978 }

979 +

980 + return;

981 }

982

983 static void graph_output_commit_line(struct git_graph *graph, struct graph_line *line)

t/t4214-log-graph-octopus.sh

+5 -5

@@ -121,7 +121,7 @@ test_expect_success 'log --graph with normal octopus merge and child, no color'

121 test_cmp expect.uncolored actual

122 '

123

124 -test_expect_failure 'log --graph with normal octopus and child merge with colors' '

124 +test_expect_success 'log --graph with normal octopus and child merge with colors' '

125 cat >expect.colors <<-\EOF &&

126 * after-merge

127 *<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET> octopus-merge

@@ -161,7 +161,7 @@ test_expect_success 'log --graph with tricky octopus merge and its child, no col

161 test_cmp expect.uncolored actual

162 '

163

164 -test_expect_failure 'log --graph with tricky octopus merge and its child with colors' '

164 +test_expect_success 'log --graph with tricky octopus merge and its child with colors' '

165 test_config log.graphColors red,green,yellow,blue,magenta,cyan &&

166 cat >expect.colors <<-\EOF &&

167 * left

@@ -205,7 +205,7 @@ test_expect_success 'log --graph with crossover in octopus merge, no color' '

205 test_cmp expect.uncolored actual

206 '

207

208 -test_expect_failure 'log --graph with crossover in octopus merge with colors' '

208 +test_expect_success 'log --graph with crossover in octopus merge with colors' '

209 test_config log.graphColors red,green,yellow,blue,magenta,cyan &&

210 cat >expect.colors <<-\EOF &&

211 * after-4

@@ -253,7 +253,7 @@ test_expect_success 'log --graph with crossover in octopus merge and its child,

253 test_cmp expect.uncolored actual

254 '

255

256 -test_expect_failure 'log --graph with crossover in octopus merge and its child with colors' '

256 +test_expect_success 'log --graph with crossover in octopus merge and its child with colors' '

257 test_config log.graphColors red,green,yellow,blue,magenta,cyan &&

258 cat >expect.colors <<-\EOF &&

259 * after-4

@@ -349,7 +349,7 @@ test_expect_success 'log --graph with unrelated commit and octopus child, no col

349 test_cmp expect.uncolored actual

350 '

351

352 -test_expect_failure 'log --graph with unrelated commit and octopus child with colors' '

352 +test_expect_success 'log --graph with unrelated commit and octopus child with colors' '

353 test_config log.graphColors red,green,yellow,blue,magenta,cyan &&

354 cat >expect.colors <<-\EOF &&

355 * after-initial