log: fix coloring of certain octopus merge shapes

For octopus merges where the first parent edge immediately merges into the next column to the left, the number of columns should be one less than the usual case. First parent to the left case: | *-. | |\ \ |/ / / The usual case: | *-. | |\ \ | | | * Also refactor the code to iterate over columns rather than dashes, building from an initial patch suggested by Jeff King. Signed-off-by: Noam Postavsky <npostavs@users.sourceforge.net> Reviewed-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Noam Postavsky committed Sep 1, 2018 at 20:07 UTC 04005834ed07e0b3a63793f386570c8bcf73f9a3
2 files changed +145 -15
graph.c
+43 -15
@@ -846,27 +846,55 @@ static void graph_output_commit_char(struct git_graph *graph, struct strbuf *sb)
846 }
847
848 /*
849 - * Draw an octopus merge and return the number of characters written.
849 + * Draw the horizontal dashes of an octopus merge and return the number of
850 + * characters written.
851 */
852 static int graph_draw_octopus_merge(struct git_graph *graph,
853 struct strbuf *sb)
854 {
855 /*
855 - * Here dashless_commits represents the number of parents
856 - * which don't need to have dashes (because their edges fit
857 - * neatly under the commit).
858 - */
859 - const int dashless_commits = 2;
860 - int col_num, i;
861 - int num_dashes =
862 - ((graph->num_parents - dashless_commits) * 2) - 1;
863 - for (i = 0; i < num_dashes; i++) {
864 - col_num = (i / 2) + dashless_commits + graph->commit_index;
865 - strbuf_write_column(sb, &graph->new_columns[col_num], '-');
856 + * Here dashless_parents represents the number of parents which don't
857 + * need to have dashes (the edges labeled "0" and "1"). And
858 + * dashful_parents are the remaining ones.
859 + *
860 + * | *---.
861 + * | |\ \ \
862 + * | | | | |
863 + * x 0 1 2 3
864 + *
865 + */
866 + const int dashless_parents = 2;
867 + int dashful_parents = graph->num_parents - dashless_parents;
868 +
869 + /*
870 + * Usually, we add one new column for each parent (like the diagram
871 + * above) but sometimes the first parent goes into an existing column,
872 + * like this:
873 + *
874 + * | *---.
875 + * | |\ \ \
876 + * |/ / / /
877 + * x 0 1 2
878 + *
879 + * In which case the number of parents will be one greater than the
880 + * number of added columns.
881 + */
882 + int added_cols = (graph->num_new_columns - graph->num_columns);
883 + int parent_in_old_cols = graph->num_parents - added_cols;
884 +
885 + /*
886 + * In both cases, commit_index corresponds to the edge labeled "0".
887 + */
888 + int first_col = graph->commit_index + dashless_parents
889 + - parent_in_old_cols;
890 +
891 + int i;
892 + for (i = 0; i < dashful_parents; i++) {
893 + strbuf_write_column(sb, &graph->new_columns[i+first_col], '-');
894 + strbuf_write_column(sb, &graph->new_columns[i+first_col],
895 + i == dashful_parents-1 ? '.' : '-');
896 }
867 - col_num = (i / 2) + dashless_commits + graph->commit_index;
868 - strbuf_write_column(sb, &graph->new_columns[col_num], '.');
869 - return num_dashes + 1;
897 + return 2 * dashful_parents;
898 }
899
900 static void graph_output_commit_line(struct git_graph *graph, struct strbuf *sb)
t/t4214-log-graph-octopus.sh new
+102
@@ -0,0 +1,102 @@
1 +#!/bin/sh
2 +
3 +test_description='git log --graph of skewed left octopus merge.'
4 +
5 +. ./test-lib.sh
6 +
7 +test_expect_success 'set up merge history' '
8 + cat >expect.uncolored <<-\EOF &&
9 + * left
10 + | *---. octopus-merge
11 + | |\ \ \
12 + |/ / / /
13 + | | | * 4
14 + | | * | 3
15 + | | |/
16 + | * | 2
17 + | |/
18 + * | 1
19 + |/
20 + * initial
21 + EOF
22 + cat >expect.colors <<-\EOF &&
23 + * left
24 + <RED>|<RESET> *<BLUE>-<RESET><BLUE>-<RESET><MAGENTA>-<RESET><MAGENTA>.<RESET> octopus-merge
25 + <RED>|<RESET> <RED>|<RESET><YELLOW>\<RESET> <BLUE>\<RESET> <MAGENTA>\<RESET>
26 + <RED>|<RESET><RED>/<RESET> <YELLOW>/<RESET> <BLUE>/<RESET> <MAGENTA>/<RESET>
27 + <RED>|<RESET> <YELLOW>|<RESET> <BLUE>|<RESET> * 4
28 + <RED>|<RESET> <YELLOW>|<RESET> * <MAGENTA>|<RESET> 3
29 + <RED>|<RESET> <YELLOW>|<RESET> <MAGENTA>|<RESET><MAGENTA>/<RESET>
30 + <RED>|<RESET> * <MAGENTA>|<RESET> 2
31 + <RED>|<RESET> <MAGENTA>|<RESET><MAGENTA>/<RESET>
32 + * <MAGENTA>|<RESET> 1
33 + <MAGENTA>|<RESET><MAGENTA>/<RESET>
34 + * initial
35 + EOF
36 + test_commit initial &&
37 + for i in 1 2 3 4 ; do
38 + git checkout master -b $i || return $?
39 + # Make tag name different from branch name, to avoid
40 + # ambiguity error when calling checkout.
41 + test_commit $i $i $i tag$i || return $?
42 + done &&
43 + git checkout 1 -b merge &&
44 + test_tick &&
45 + git merge -m octopus-merge 1 2 3 4 &&
46 + git checkout 1 -b L &&
47 + test_commit left
48 +'
49 +
50 +test_expect_success 'log --graph with tricky octopus merge with colors' '
51 + test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
52 + git log --color=always --graph --date-order --pretty=tformat:%s --all >actual.colors.raw &&
53 + test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
54 + test_cmp expect.colors actual.colors
55 +'
56 +
57 +test_expect_success 'log --graph with tricky octopus merge, no color' '
58 + git log --color=never --graph --date-order --pretty=tformat:%s --all >actual.raw &&
59 + sed "s/ *\$//" actual.raw >actual &&
60 + test_cmp expect.uncolored actual
61 +'
62 +
63 +# Repeat the previous two tests with "normal" octopus merge (i.e.,
64 +# without the first parent skewing to the "left" branch column).
65 +
66 +test_expect_success 'log --graph with normal octopus merge, no color' '
67 + cat >expect.uncolored <<-\EOF &&
68 + *---. octopus-merge
69 + |\ \ \
70 + | | | * 4
71 + | | * | 3
72 + | | |/
73 + | * | 2
74 + | |/
75 + * | 1
76 + |/
77 + * initial
78 + EOF
79 + git log --color=never --graph --date-order --pretty=tformat:%s merge >actual.raw &&
80 + sed "s/ *\$//" actual.raw >actual &&
81 + test_cmp expect.uncolored actual
82 +'
83 +
84 +test_expect_success 'log --graph with normal octopus merge with colors' '
85 + cat >expect.colors <<-\EOF &&
86 + *<YELLOW>-<RESET><YELLOW>-<RESET><BLUE>-<RESET><BLUE>.<RESET> octopus-merge
87 + <RED>|<RESET><GREEN>\<RESET> <YELLOW>\<RESET> <BLUE>\<RESET>
88 + <RED>|<RESET> <GREEN>|<RESET> <YELLOW>|<RESET> * 4
89 + <RED>|<RESET> <GREEN>|<RESET> * <BLUE>|<RESET> 3
90 + <RED>|<RESET> <GREEN>|<RESET> <BLUE>|<RESET><BLUE>/<RESET>
91 + <RED>|<RESET> * <BLUE>|<RESET> 2
92 + <RED>|<RESET> <BLUE>|<RESET><BLUE>/<RESET>
93 + * <BLUE>|<RESET> 1
94 + <BLUE>|<RESET><BLUE>/<RESET>
95 + * initial
96 + EOF
97 + test_config log.graphColors red,green,yellow,blue,magenta,cyan &&
98 + git log --color=always --graph --date-order --pretty=tformat:%s merge >actual.colors.raw &&
99 + test_decode_color <actual.colors.raw | sed "s/ *\$//" >actual.colors &&
100 + test_cmp expect.colors actual.colors
101 +'
102 +test_done