Skip to content

Commit dd119c7

Browse files
gitstergitster
committed
Merge branch 'ps/shift-root-in-graph' into jch
In a history with more than one root commit, "git log --graph --oneline" stuffed an unrelated commit immediately below a root commit, which has been corrected by making the spot below a root unavailable. * ps/shift-root-in-graph: graph: add indentation for commits preceded by a parentless commit
2 parents e1442df + eecc860 commit dd119c7

2 files changed

Lines changed: 233 additions & 6 deletions

File tree

‎graph.c‎

Lines changed: 109 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ struct column {
6060
* index into column_colors.
6161
*/
6262
unsigned short color;
63+
/*
64+
* A placeholder column keeps the column of a parentless commit filled
65+
* for one extra row, avoiding a next unrelated commit to be printed
66+
* in the same column.
67+
*/
68+
unsigned is_placeholder:1;
6369
};
6470

6571
enum graph_state {
@@ -572,6 +578,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
572578
i = graph->num_new_columns++;
573579
graph->new_columns[i].commit = commit;
574580
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
581+
graph->new_columns[i].is_placeholder = 0;
575582
}
576583

577584
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -616,7 +623,7 @@ static void graph_update_columns(struct git_graph *graph)
616623
{
617624
struct commit_list *parent;
618625
int max_new_columns;
619-
int i, seen_this, is_commit_in_columns;
626+
int i, seen_this, is_commit_in_columns, is_parentless;
620627

621628
/*
622629
* Swap graph->columns with graph->new_columns
@@ -663,6 +670,26 @@ static void graph_update_columns(struct git_graph *graph)
663670
*/
664671
seen_this = 0;
665672
is_commit_in_columns = 1;
673+
/*
674+
* A commit is "parentless" (is a visual root that starts a new column)
675+
* only if has no visible parents AND it's not a boundary commit.
676+
*
677+
* Boundary commits also have no visible parents, but they are
678+
* NOT a visual root:
679+
*
680+
* 1. A boundary only appears in the output because an included commit
681+
* is its child. Children are always above, and the renderer draws an
682+
* edge down to the boundary from that child. Rather than starting
683+
* a column like a visual root would do, it "inherits" its child
684+
* column.
685+
*
686+
* 2. Included commit CAN'T appear below a boundary. Boundaries are
687+
* ancestors of the exclusion point; if an included commit were an
688+
* ancestor of the boundary it would be excluded and not rendered.
689+
* Boundaries therefore always sink to the bottom.
690+
*/
691+
is_parentless = graph->num_parents == 0 &&
692+
!(graph->commit->object.flags & BOUNDARY);
666693
for (i = 0; i <= graph->num_columns; i++) {
667694
struct commit *col_commit;
668695
if (i == graph->num_columns) {
@@ -697,11 +724,46 @@ static void graph_update_columns(struct git_graph *graph)
697724
* least 2, even if it has no interesting parents.
698725
* The current commit always takes up at least 2
699726
* spaces.
727+
*
728+
* Check for the commit to seem like a root, no parents
729+
* rendered and that it is not a boundary commit. If so,
730+
* add a placeholder to keep that column filled for
731+
* at least one row.
732+
*
733+
* Prevents the next commit from being inserted
734+
* just below and making the graph confusing.
700735
*/
701-
if (graph->num_parents == 0)
736+
if (is_parentless) {
737+
graph_insert_into_new_columns(graph, graph->commit, i);
738+
graph->new_columns[graph->num_new_columns - 1]
739+
.is_placeholder = 1;
740+
} else if (graph->num_parents == 0) {
702741
graph->width += 2;
742+
}
703743
} else {
704-
graph_insert_into_new_columns(graph, col_commit, -1);
744+
if (graph->columns[i].is_placeholder) {
745+
/*
746+
* Keep the placeholders if the next commit is
747+
* parentless also, making the indentation cascade.
748+
*/
749+
if (!seen_this && is_parentless) {
750+
graph_insert_into_new_columns(graph,
751+
graph->columns[i].commit, i);
752+
graph->new_columns[graph->num_new_columns - 1]
753+
.is_placeholder = 1;
754+
} else if (!seen_this) {
755+
graph->mapping[graph->width] = -1;
756+
graph->width += 2;
757+
}
758+
/*
759+
* seen_this && is_placeholder means that this
760+
* line is the one after the indented one, the
761+
* placeholder is no longer needed, gets
762+
* dropped and the columns collapses naturally.
763+
*/
764+
} else {
765+
graph_insert_into_new_columns(graph, col_commit, -1);
766+
}
705767
}
706768
}
707769

@@ -872,7 +934,10 @@ static void graph_output_padding_line(struct git_graph *graph,
872934
graph_line_addstr(line, "~ ");
873935
break;
874936
}
875-
graph_line_write_column(line, &graph->new_columns[i], '|');
937+
if (graph->new_columns[i].is_placeholder)
938+
graph_line_write_column(line, &graph->new_columns[i], ' ');
939+
else
940+
graph_line_write_column(line, &graph->new_columns[i], '|');
876941
graph_line_addch(line, ' ');
877942
}
878943
}
@@ -1106,7 +1171,34 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
11061171
graph->mapping[2 * i] < i) {
11071172
graph_line_write_column(line, col, '/');
11081173
} else {
1109-
graph_line_write_column(line, col, '|');
1174+
if (col->is_placeholder) {
1175+
/*
1176+
* When the indented commit is a merge commit,
1177+
* the placeholder column adds unwanted padding
1178+
* between the commit and its subject.
1179+
*
1180+
* * parentless commit
1181+
* * merge commit
1182+
* /|
1183+
* | * parent A
1184+
* * parent B
1185+
* ^^ unwanted padding
1186+
*
1187+
* Once the current commit has been seen, don't
1188+
* let placeholder columns to be rendered:
1189+
*
1190+
* * parentless commit
1191+
* * merge commit
1192+
* /|
1193+
* | * parent A
1194+
* * parent B
1195+
*/
1196+
if (seen_this)
1197+
continue;
1198+
graph_line_write_column(line, col, ' ');
1199+
} else {
1200+
graph_line_write_column(line, col, '|');
1201+
}
11101202
}
11111203
graph_line_addch(line, ' ');
11121204
}
@@ -1250,7 +1342,18 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
12501342
if (!graph_needs_truncation(graph, i + 1))
12511343
graph_line_addch(line, ' ');
12521344
} else {
1253-
graph_line_write_column(line, col, '|');
1345+
if (col->is_placeholder) {
1346+
/*
1347+
* Same placeholder handling as in
1348+
* graph_output_commit_line().
1349+
*/
1350+
if (seen_this)
1351+
continue;
1352+
graph_line_write_column(line, col, ' ');
1353+
} else {
1354+
graph_line_write_column(line, col, '|');
1355+
}
1356+
12541357
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
12551358
if (parent_col)
12561359
graph_line_write_column(

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,4 +514,128 @@ test_expect_success 'log --graph --graph-lane-limit=7 check if it shows all 3 pa
514514
EOF
515515
'
516516

517+
test_expect_success 'log --graph with root commit' '
518+
git checkout --orphan 8_1 && test_commit 8_A && test_commit 8_A1 &&
519+
git checkout --orphan 8_2 && test_commit 8_B &&
520+
521+
check_graph 8_2 8_1 <<-\EOF
522+
* 8_B
523+
* 8_A1
524+
/
525+
* 8_A
526+
EOF
527+
'
528+
529+
test_expect_success 'log --graph with multiple root commits' '
530+
test_commit 8_B1 &&
531+
git checkout --orphan 8_3 && test_commit 8_C &&
532+
533+
check_graph 8_3 8_2 8_1 <<-\EOF
534+
* 8_C
535+
* 8_B1
536+
/
537+
* 8_B
538+
* 8_A1
539+
/
540+
* 8_A
541+
EOF
542+
'
543+
544+
test_expect_success 'log --graph commit from a two parent merge shifted' '
545+
git checkout --orphan 9_1 && test_commit 9_B &&
546+
git checkout --orphan 9_2 && test_commit 9_C &&
547+
git checkout 9_1 &&
548+
git merge 9_2 --allow-unrelated-histories -m 9_M &&
549+
git checkout --orphan 9_3 &&
550+
test_commit 9_A && test_commit 9_A1 && test_commit 9_A2 &&
551+
552+
check_graph 9_3 9_1 <<-\EOF
553+
* 9_A2
554+
* 9_A1
555+
* 9_A
556+
* 9_M
557+
/|
558+
| * 9_C
559+
* 9_B
560+
EOF
561+
'
562+
563+
test_expect_success 'log --graph commit from a three parent merge shifted' '
564+
git checkout --orphan 10_1 && test_commit 10_B &&
565+
git checkout --orphan 10_2 && test_commit 10_C &&
566+
git checkout --orphan 10_3 && test_commit 10_D &&
567+
git checkout 10_1 &&
568+
TREE=$(git write-tree) &&
569+
MERGE=$(git commit-tree $TREE -p 10_1 -p 10_2 -p 10_3 -m 10_M) &&
570+
git reset --hard $MERGE &&
571+
git checkout --orphan 10_4 &&
572+
test_commit 10_A && test_commit 10_A1 && test_commit 10_A2 &&
573+
574+
check_graph 10_4 10_1 <<-\EOF
575+
* 10_A2
576+
* 10_A1
577+
* 10_A
578+
* 10_M
579+
/|\
580+
| | * 10_D
581+
| * 10_C
582+
* 10_B
583+
EOF
584+
'
585+
586+
test_expect_success 'log --graph commit from a four parent merge shifted' '
587+
git checkout --orphan 11_1 && test_commit 11_B &&
588+
git checkout --orphan 11_2 && test_commit 11_C &&
589+
git checkout --orphan 11_3 && test_commit 11_D &&
590+
git checkout --orphan 11_4 && test_commit 11_E &&
591+
git checkout 11_1 &&
592+
TREE=$(git write-tree) &&
593+
MERGE=$(git commit-tree $TREE -p 11_1 -p 11_2 -p 11_3 -p 11_4 -m 11_M) &&
594+
git reset --hard $MERGE &&
595+
git checkout --orphan 11_5 &&
596+
test_commit 11_A && test_commit 11_A1 && test_commit 11_A2 &&
597+
598+
check_graph 11_5 11_1 <<-\EOF
599+
* 11_A2
600+
* 11_A1
601+
* 11_A
602+
*-. 11_M
603+
/|\ \
604+
| | | * 11_E
605+
| | * 11_D
606+
| * 11_C
607+
* 11_B
608+
EOF
609+
'
610+
611+
test_expect_success 'log --graph disconnected three roots cascading' '
612+
git checkout --orphan 12_1 && test_commit 12_D && test_commit 12_D1 &&
613+
git checkout --orphan 12_2 && test_commit 12_C &&
614+
git checkout --orphan 12_3 && test_commit 12_B &&
615+
git checkout --orphan 12_4 && test_commit 12_A &&
616+
617+
check_graph 12_4 12_3 12_2 12_1 <<-\EOF
618+
* 12_A
619+
* 12_B
620+
* 12_C
621+
* 12_D1
622+
_ /
623+
/
624+
/
625+
* 12_D
626+
EOF
627+
'
628+
629+
test_expect_success 'log --graph with excluded parent (not a root)' '
630+
git checkout --orphan 13_1 && test_commit 13_X && test_commit 13_Y &&
631+
git checkout --orphan 13_2 && test_commit 13_O && test_commit 13_A &&
632+
633+
check_graph 13_O..13_A 13_1 <<-\EOF
634+
* 13_A
635+
* 13_Y
636+
/
637+
* 13_X
638+
EOF
639+
'
640+
517641
test_done

0 commit comments

Comments
 (0)