Skip to content

Commit d6e1ee4

Browse files
pabloosabaterrgitster
pabloosabaterr
authored and
gitster
committed
graph: add indentation for commits preceded by a root
When having a history with multiple root commits and drawing the history near the roots the graphing engine renders the commit one below the other, seeming that they are related. This issue was reported by Junio at: https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/ This happens because the root has no parents thus for the next row printing, the column becomes free and the engine prints from the first free columns left to right. Keep the root commit at least one row more to avoid having the column empty but hide it, therefore making the next unrelated commit to live in the next column (column means even positions where edges live: 0, 2, 4), then clean that "placeholder" column and let the unrelated commit to naturally collapse to the column where the root commit was. Add is_placeholder to the struct column to mark if a column is acting as a placeholder for the padding. When the column is a root, add a column with the root commit data to prevent segfaults when 'column->commit' and mark it as a placeholder. After, unless the next commit is also a root (then we need to keep cascading the indentation) clean the mapping and the columns from the placeholder to allow it to collapse naturally. Teach rendering functions to print a padding ' ' when a placeholder column is met. Add tests for different cases. before this patch: * root-B * child-A2 * child-A1 * root-A after this patch: * root-B * child-A2 / * child-A1 * root-A Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 2565546 commit d6e1ee4

File tree

2 files changed

+198
-6
lines changed

2 files changed

+198
-6
lines changed

‎graph.c‎

Lines changed: 62 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,13 @@ struct column {
6060
* index into column_colors.
6161
*/
6262
unsigned short color;
63+
/*
64+
* A placeholder column keeps the column of the root filled for one
65+
* extra row, avoiding a next unrelated commit to be printed in the
66+
* same column. Placeholder columns don't propagate to the following
67+
* commit.
68+
*/
69+
unsigned is_placeholder:1;
6370
};
6471

6572
enum graph_state {
@@ -563,6 +570,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
563570
i = graph->num_new_columns++;
564571
graph->new_columns[i].commit = commit;
565572
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
573+
graph->new_columns[i].is_placeholder = 0;
566574
}
567575

568576
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -607,7 +615,7 @@ static void graph_update_columns(struct git_graph *graph)
607615
{
608616
struct commit_list *parent;
609617
int max_new_columns;
610-
int i, seen_this, is_commit_in_columns;
618+
int i, seen_this, is_commit_in_columns, is_root;
611619

612620
/*
613621
* Swap graph->columns with graph->new_columns
@@ -654,6 +662,9 @@ static void graph_update_columns(struct git_graph *graph)
654662
*/
655663
seen_this = 0;
656664
is_commit_in_columns = 1;
665+
is_root = graph->num_parents == 0 &&
666+
!graph->commit->parents &&
667+
!(graph->commit->object.flags & BOUNDARY);
657668
for (i = 0; i <= graph->num_columns; i++) {
658669
struct commit *col_commit;
659670
if (i == graph->num_columns) {
@@ -688,11 +699,40 @@ static void graph_update_columns(struct git_graph *graph)
688699
* least 2, even if it has no interesting parents.
689700
* The current commit always takes up at least 2
690701
* spaces.
702+
*
703+
* Check for the commit to be a root, no parents
704+
* and that it is not a boundary commit. If so, add a
705+
* placeholder to keep that column filled for
706+
* at least one row.
707+
*
708+
* Prevents the next commit from being inserted
709+
* just below and making the graph confusing.
691710
*/
692-
if (graph->num_parents == 0)
711+
if (is_root) {
712+
graph_insert_into_new_columns(graph, graph->commit, i);
713+
graph->new_columns[graph->num_new_columns - 1]
714+
.is_placeholder = 1;
715+
} else if (graph->num_parents == 0) {
693716
graph->width += 2;
717+
}
694718
} else {
695-
graph_insert_into_new_columns(graph, col_commit, -1);
719+
if (graph->columns[i].is_placeholder) {
720+
/*
721+
* Keep the placeholders if the next commit is
722+
* a root also, making the indentation cascade.
723+
*/
724+
if (!seen_this && is_root) {
725+
graph_insert_into_new_columns(graph,
726+
graph->columns[i].commit, i);
727+
graph->new_columns[graph->num_new_columns - 1]
728+
.is_placeholder = 1;
729+
} else if (!seen_this) {
730+
graph->mapping[graph->width] = -1;
731+
graph->width += 2;
732+
}
733+
} else {
734+
graph_insert_into_new_columns(graph, col_commit, -1);
735+
}
696736
}
697737
}
698738

@@ -846,7 +886,10 @@ static void graph_output_padding_line(struct git_graph *graph,
846886
* Output a padding row, that leaves all branch lines unchanged
847887
*/
848888
for (i = 0; i < graph->num_new_columns; i++) {
849-
graph_line_write_column(line, &graph->new_columns[i], '|');
889+
if (graph->new_columns[i].is_placeholder)
890+
graph_line_write_column(line, &graph->new_columns[i], ' ');
891+
else
892+
graph_line_write_column(line, &graph->new_columns[i], '|');
850893
graph_line_addch(line, ' ');
851894
}
852895
}
@@ -1058,7 +1101,13 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
10581101
graph->mapping[2 * i] < i) {
10591102
graph_line_write_column(line, col, '/');
10601103
} else {
1061-
graph_line_write_column(line, col, '|');
1104+
if (col->is_placeholder) {
1105+
if (seen_this)
1106+
continue;
1107+
graph_line_write_column(line, col, ' ');
1108+
} else {
1109+
graph_line_write_column(line, col, '|');
1110+
}
10621111
}
10631112
graph_line_addch(line, ' ');
10641113
}
@@ -1135,7 +1184,14 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
11351184
graph_line_write_column(line, col, '|');
11361185
graph_line_addch(line, ' ');
11371186
} else {
1138-
graph_line_write_column(line, col, '|');
1187+
if (col->is_placeholder) {
1188+
if (seen_this)
1189+
continue;
1190+
graph_line_write_column(line, col, ' ');
1191+
} else {
1192+
graph_line_write_column(line, col, '|');
1193+
}
1194+
11391195
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
11401196
if (parent_col)
11411197
graph_line_write_column(

‎t/t4215-log-skewed-merges.sh‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,4 +370,140 @@ test_expect_success 'log --graph with multiple tips' '
370370
EOF
371371
'
372372

373+
test_expect_success 'log --graph with root commit' '
374+
git checkout --orphan 8_a &&
375+
test_commit 8_A &&
376+
test_commit 8_A1 &&
377+
git checkout --orphan 8_b &&
378+
test_commit 8_B &&
379+
380+
check_graph 8_b 8_a <<-\EOF
381+
* 8_B
382+
* 8_A1
383+
/
384+
* 8_A
385+
EOF
386+
'
387+
388+
test_expect_success 'log --graph with multiple root commits' '
389+
test_commit 8_B1 &&
390+
git checkout --orphan 8_c &&
391+
test_commit 8_C &&
392+
393+
check_graph 8_c 8_b 8_a <<-\EOF
394+
* 8_C
395+
* 8_B1
396+
/
397+
* 8_B
398+
* 8_A1
399+
/
400+
* 8_A
401+
EOF
402+
'
403+
404+
test_expect_success 'log --graph commit from a two parent merge shifted' '
405+
git checkout --orphan 9_b &&
406+
test_commit 9_B &&
407+
git checkout --orphan 9_c &&
408+
test_commit 9_C &&
409+
git checkout 9_b &&
410+
git merge 9_c --allow-unrelated-histories -m 9_M &&
411+
git checkout --orphan 9_a &&
412+
test_commit 9_A &&
413+
test_commit 9_A1 &&
414+
test_commit 9_A2 &&
415+
416+
check_graph 9_a 9_b <<-\EOF
417+
* 9_A2
418+
* 9_A1
419+
* 9_A
420+
* 9_M
421+
/|
422+
| * 9_C
423+
* 9_B
424+
EOF
425+
'
426+
427+
test_expect_success 'log --graph commit from a three parent merge shifted' '
428+
git checkout --orphan 10_b &&
429+
test_commit 10_B &&
430+
git checkout --orphan 10_c &&
431+
test_commit 10_C &&
432+
git checkout --orphan 10_d &&
433+
test_commit 10_D &&
434+
git checkout 10_b &&
435+
TREE=$(git write-tree) &&
436+
MERGE=$(git commit-tree $TREE -p 10_b -p 10_c -p 10_d -m 10_M) &&
437+
git reset --hard $MERGE &&
438+
git checkout --orphan 10_a &&
439+
test_commit 10_A &&
440+
test_commit 10_A1 &&
441+
test_commit 10_A2 &&
442+
443+
check_graph 10_a 10_b <<-\EOF
444+
* 10_A2
445+
* 10_A1
446+
* 10_A
447+
* 10_M
448+
/|\
449+
| | * 10_D
450+
| * 10_C
451+
* 10_B
452+
EOF
453+
'
454+
455+
test_expect_success 'log --graph commit from a four parent merge shifted' '
456+
git checkout --orphan 11_b &&
457+
test_commit 11_B &&
458+
git checkout --orphan 11_c &&
459+
test_commit 11_C &&
460+
git checkout --orphan 11_d &&
461+
test_commit 11_D &&
462+
git checkout --orphan 11_e &&
463+
test_commit 11_E &&
464+
git checkout 11_b &&
465+
TREE=$(git write-tree) &&
466+
MERGE=$(git commit-tree $TREE -p 11_b -p 11_c -p 11_d -p 11_e -m 11_M) &&
467+
git reset --hard $MERGE &&
468+
git checkout --orphan 11_a &&
469+
test_commit 11_A &&
470+
test_commit 11_A1 &&
471+
test_commit 11_A2 &&
472+
473+
check_graph 11_a 11_b <<-\EOF
474+
* 11_A2
475+
* 11_A1
476+
* 11_A
477+
*-. 11_M
478+
/|\ \
479+
| | | * 11_E
480+
| | * 11_D
481+
| * 11_C
482+
* 11_B
483+
EOF
484+
'
485+
486+
test_expect_success 'log --graph disconnected three roots cascading' '
487+
git checkout --orphan 12_d &&
488+
test_commit 12_D &&
489+
test_commit 12_D1 &&
490+
git checkout --orphan 12_c &&
491+
test_commit 12_C &&
492+
git checkout --orphan 12_b &&
493+
test_commit 12_B &&
494+
git checkout --orphan 12_a &&
495+
test_commit 12_A &&
496+
497+
check_graph 12_a 12_b 12_c 12_d <<-\EOF
498+
* 12_A
499+
* 12_B
500+
* 12_C
501+
* 12_D1
502+
_ /
503+
/
504+
/
505+
* 12_D
506+
EOF
507+
'
508+
373509
test_done

0 commit comments

Comments
 (0)