Skip to content

Commit ea0df1d

Browse files
jcoglanjcoglan
committed
graph: fix coloring of octopus dashes
In 0400583 ("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 <[email protected]>
1 parent 50756ed commit ea0df1d

File tree

2 files changed

+92
-38
lines changed

2 files changed

+92
-38
lines changed

‎graph.c

Lines changed: 37 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -665,6 +665,11 @@ static void graph_update_columns(struct git_graph *graph)
665665
graph->mapping_size--;
666666
}
667667

668+
static int graph_num_dashed_parents(struct git_graph *graph)
669+
{
670+
return graph->num_parents + graph->merge_layout - 3;
671+
}
672+
668673
static int graph_num_expansion_rows(struct git_graph *graph)
669674
{
670675
/*
@@ -687,7 +692,7 @@ static int graph_num_expansion_rows(struct git_graph *graph)
687692
* | * \
688693
* |/|\ \
689694
*/
690-
return (graph->num_parents + graph->merge_layout - 3) * 2;
695+
return graph_num_dashed_parents(graph) * 2;
691696
}
692697

693698
static int graph_needs_pre_commit_line(struct git_graph *graph)
@@ -930,47 +935,45 @@ static void graph_output_commit_char(struct git_graph *graph, struct strbuf *sb)
930935
static void graph_draw_octopus_merge(struct git_graph *graph, struct strbuf *sb)
931936
{
932937
/*
933-
* Here dashless_parents represents the number of parents which don't
934-
* need to have dashes (the edges labeled "0" and "1"). And
935-
* dashful_parents are the remaining ones.
938+
* The parents of a merge commit can be arbitrarily reordered as they
939+
* are mapped onto display columns, for example this is a valid merge:
936940
*
937-
* | *---.
938-
* | |\ \ \
939-
* | | | | |
940-
* x 0 1 2 3
941+
* | | *---.
942+
* | | |\ \ \
943+
* | | |/ / /
944+
* | |/| | /
945+
* | |_|_|/
946+
* |/| | |
947+
* 3 1 0 2
941948
*
942-
*/
943-
const int dashless_parents = 3 - graph->merge_layout;
944-
int dashful_parents = graph->num_parents - dashless_parents;
945-
946-
/*
947-
* Usually, we add one new column for each parent (like the diagram
948-
* above) but sometimes the first parent goes into an existing column,
949-
* like this:
949+
* The numbers denote which parent of the merge each visual column
950+
* corresponds to; we can't assume that the parents will initially
951+
* display in the order given by new_columns.
950952
*
951-
* | *-.
952-
* |/|\ \
953-
* | | | |
954-
* x 0 1 2
953+
* To find the right color for each dash, we need to consult the
954+
* mapping array, starting from the column 2 places to the right of the
955+
* merge commit, and use that to find out which logical column each
956+
* edge will collapse to.
955957
*
956-
* In which case the number of parents will be one greater than the
957-
* number of added columns.
958+
* Commits are rendered once all edges have collapsed to their correct
959+
* logcial column, so commit_index gives us the right visual offset for
960+
* the merge commit.
958961
*/
959-
int added_cols = (graph->num_new_columns - graph->num_columns);
960-
int parent_in_old_cols = graph->num_parents - added_cols;
961962

962-
/*
963-
* In both cases, commit_index corresponds to the edge labeled "0".
964-
*/
965-
int first_col = graph->commit_index + dashless_parents
966-
- parent_in_old_cols;
963+
int i, j;
964+
struct column *col;
967965

968-
int i;
969-
for (i = 0; i < dashful_parents; i++) {
970-
strbuf_write_column(sb, &graph->new_columns[i+first_col], '-');
971-
strbuf_write_column(sb, &graph->new_columns[i+first_col],
972-
i == dashful_parents-1 ? '.' : '-');
966+
int dashed_parents = graph_num_dashed_parents(graph);
967+
968+
for (i = 0; i < dashed_parents; i++) {
969+
j = graph->mapping[(graph->commit_index + i + 2) * 2];
970+
col = &graph->new_columns[j];
971+
972+
strbuf_write_column(sb, col, '-');
973+
strbuf_write_column(sb, col, (i == dashed_parents - 1) ? '.' : '-');
973974
}
975+
976+
return;
974977
}
975978

976979
static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)

‎t/t4214-log-graph-octopus.sh

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,23 +42,74 @@ test_expect_success 'set up merge history' '
4242
test_tick &&
4343
git merge -m octopus-merge 1 2 3 4 &&
4444
git checkout 1 -b L &&
45-
test_commit left
45+
test_commit left &&
46+
git checkout 4 -b R &&
47+
test_commit right
4648
'
4749

4850
test_expect_success 'log --graph with tricky octopus merge with colors' '
4951
test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
50-
git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw &&
52+
git log --color=always --graph --date-order --pretty=tformat:%s L merge >actual.colors.raw &&
5153
test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
5254
test_cmp expect.colors actual.colors
5355
'
5456

5557
test_expect_success 'log --graph with tricky octopus merge, no color' '
56-
git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
58+
git log --color=never --graph --date-order --pretty=tformat:%s L merge >actual.raw &&
5759
sed "s/ *\$//" actual.raw >actual &&
5860
test_cmp expect.uncolored actual
5961
'
6062

61-
# Repeat the previous two tests with "normal" octopus merge (i.e.,
63+
# Repeat the previous two tests with an octopus merge whose final parent skews left
64+
65+
test_expect_success 'log --graph with left-skewed final parent, no color' '
66+
cat >expect.uncolored <<-\EOF &&
67+
* right
68+
| *---. octopus-merge
69+
| |\ \ \
70+
| |_|_|/
71+
|/| | |
72+
* | | | 4
73+
| | | * 3
74+
| |_|/
75+
|/| |
76+
| | * 2
77+
| |/
78+
|/|
79+
| * 1
80+
|/
81+
* initial
82+
EOF
83+
git log --color=never --graph --date-order --pretty=tformat:%s R merge >actual.raw &&
84+
sed "s/ *\$//" actual.raw >actual &&
85+
test_cmp expect.uncolored actual
86+
'
87+
88+
test_expect_success 'log --graph with left-skewed final parent with colors' '
89+
cat >expect.colors <<-\EOF &&
90+
* right
91+
<RED>|<RESET> *<BLUE>-<RESET><BLUE>-<RESET><RED>-<RESET><RED>.<RESET> octopus-merge
92+
<RED>|<RESET> <GREEN>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> <RED>\<RESET>
93+
<RED>|<RESET> <GREEN>|<RESET><RED>_<RESET><YELLOW>|<RESET><RED>_<RESET><BLUE>|<RESET><RED>/<RESET>
94+
<RED>|<RESET><RED>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET>
95+
* <GREEN>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> 4
96+
<MAGENTA>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> * 3
97+
<MAGENTA>|<RESET> <GREEN>|<RESET><MAGENTA>_<RESET><YELLOW>|<RESET><MAGENTA>/<RESET>
98+
<MAGENTA>|<RESET><MAGENTA>/<RESET><GREEN>|<RESET> <YELLOW>|<RESET>
99+
<MAGENTA>|<RESET> <GREEN>|<RESET> * 2
100+
<MAGENTA>|<RESET> <GREEN>|<RESET><MAGENTA>/<RESET>
101+
<MAGENTA>|<RESET><MAGENTA>/<RESET><GREEN>|<RESET>
102+
<MAGENTA>|<RESET> * 1
103+
<MAGENTA>|<RESET><MAGENTA>/<RESET>
104+
* initial
105+
EOF
106+
test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
107+
git log --color=always --graph --date-order --pretty=tformat:%s R merge >actual.colors.raw &&
108+
test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
109+
test_cmp expect.colors actual.colors
110+
'
111+
112+
# Repeat the first two tests with "normal" octopus merge (i.e.,
62113
# without the first parent skewing to the "left" branch column).
63114

64115
test_expect_success 'log --graph with normal octopus merge, no color' '

0 commit comments

Comments
 (0)