Skip to content

Commit b8fb488

Browse files
pabloosabaterrgitster
pabloosabaterr
authored and
gitster
committed
graph: add indentation for commits preceded by a parentless commit
When having a history with multiple root commits or commits that act like roots (they have excluded parents), let's call them parentless, and drawing the history near them, the graphing engine renders the commits one below the other, seeming that they are related. This issue has been attempted multiple times: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/ This happens because for these parentless commits, in the next row the column becomes empty and the engine prints from left to right from the first empty column, filling the gap below these parentless commits. Keep a parentless commit for at least one row more to avoid having the column empty but hide it as indentation, therefore making the next unrelated commit 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 parentless commit was. Add is_placeholder to the struct column to mark if a column is acting as a placeholder for the padding. When a column is parentless, add a column with the parentless commit data to prevent segfaults when 'column->commit' and mark it as a placeholder. Teach rendering functions to print a padding ' ' instead of an edge when a placeholder column is met. Then, unless the next commit is also parentless (then we need to keep cascading the indentation) clean the mapping and columns from the placeholder to allow it to collapse naturally. Add tests for different cases. before this patch: * parentless-B * child-A2 * child-A1 * parentless-A after this patch: * parentless-B * child-A2 / * child-A1 * parentless-A Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
1 parent 2565546 commit b8fb488

File tree

2 files changed

+188
-6
lines changed

2 files changed

+188
-6
lines changed

‎graph.c‎

Lines changed: 64 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 {
@@ -563,6 +569,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
563569
i = graph->num_new_columns++;
564570
graph->new_columns[i].commit = commit;
565571
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
572+
graph->new_columns[i].is_placeholder = 0;
566573
}
567574

568575
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -607,7 +614,7 @@ static void graph_update_columns(struct git_graph *graph)
607614
{
608615
struct commit_list *parent;
609616
int max_new_columns;
610-
int i, seen_this, is_commit_in_columns;
617+
int i, seen_this, is_commit_in_columns, seems_root;
611618

612619
/*
613620
* Swap graph->columns with graph->new_columns
@@ -654,6 +661,12 @@ static void graph_update_columns(struct git_graph *graph)
654661
*/
655662
seen_this = 0;
656663
is_commit_in_columns = 1;
664+
/*
665+
* num_parents == 0 means that there are no parents flagged as
666+
* interesting to being shown.
667+
*/
668+
seems_root = graph->num_parents == 0 &&
669+
!(graph->commit->object.flags & BOUNDARY);
657670
for (i = 0; i <= graph->num_columns; i++) {
658671
struct commit *col_commit;
659672
if (i == graph->num_columns) {
@@ -688,11 +701,40 @@ static void graph_update_columns(struct git_graph *graph)
688701
* least 2, even if it has no interesting parents.
689702
* The current commit always takes up at least 2
690703
* spaces.
704+
*
705+
* Check for the commit to seem like a root, no parents
706+
* rendered and that it is not a boundary commit. If so,
707+
* add a placeholder to keep that column filled for
708+
* at least one row.
709+
*
710+
* Prevents the next commit from being inserted
711+
* just below and making the graph confusing.
691712
*/
692-
if (graph->num_parents == 0)
713+
if (seems_root) {
714+
graph_insert_into_new_columns(graph, graph->commit, i);
715+
graph->new_columns[graph->num_new_columns - 1]
716+
.is_placeholder = 1;
717+
} else if (graph->num_parents == 0) {
693718
graph->width += 2;
719+
}
694720
} else {
695-
graph_insert_into_new_columns(graph, col_commit, -1);
721+
if (graph->columns[i].is_placeholder) {
722+
/*
723+
* Keep the placeholders if the next commit is
724+
* parentless also, making the indentation cascade.
725+
*/
726+
if (!seen_this && seems_root) {
727+
graph_insert_into_new_columns(graph,
728+
graph->columns[i].commit, i);
729+
graph->new_columns[graph->num_new_columns - 1]
730+
.is_placeholder = 1;
731+
} else if (!seen_this) {
732+
graph->mapping[graph->width] = -1;
733+
graph->width += 2;
734+
}
735+
} else {
736+
graph_insert_into_new_columns(graph, col_commit, -1);
737+
}
696738
}
697739
}
698740

@@ -846,7 +888,10 @@ static void graph_output_padding_line(struct git_graph *graph,
846888
* Output a padding row, that leaves all branch lines unchanged
847889
*/
848890
for (i = 0; i < graph->num_new_columns; i++) {
849-
graph_line_write_column(line, &graph->new_columns[i], '|');
891+
if (graph->new_columns[i].is_placeholder)
892+
graph_line_write_column(line, &graph->new_columns[i], ' ');
893+
else
894+
graph_line_write_column(line, &graph->new_columns[i], '|');
850895
graph_line_addch(line, ' ');
851896
}
852897
}
@@ -1058,7 +1103,13 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
10581103
graph->mapping[2 * i] < i) {
10591104
graph_line_write_column(line, col, '/');
10601105
} else {
1061-
graph_line_write_column(line, col, '|');
1106+
if (col->is_placeholder) {
1107+
if (seen_this)
1108+
continue;
1109+
graph_line_write_column(line, col, ' ');
1110+
} else {
1111+
graph_line_write_column(line, col, '|');
1112+
}
10621113
}
10631114
graph_line_addch(line, ' ');
10641115
}
@@ -1135,7 +1186,14 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
11351186
graph_line_write_column(line, col, '|');
11361187
graph_line_addch(line, ' ');
11371188
} else {
1138-
graph_line_write_column(line, col, '|');
1189+
if (col->is_placeholder) {
1190+
if (seen_this)
1191+
continue;
1192+
graph_line_write_column(line, col, ' ');
1193+
} else {
1194+
graph_line_write_column(line, col, '|');
1195+
}
1196+
11391197
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
11401198
if (parent_col)
11411199
graph_line_write_column(

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

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -370,4 +370,128 @@ 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_1 && test_commit 8_A && test_commit 8_A1 &&
375+
git checkout --orphan 8_2 && test_commit 8_B &&
376+
377+
check_graph 8_2 8_1 <<-\EOF
378+
* 8_B
379+
* 8_A1
380+
/
381+
* 8_A
382+
EOF
383+
'
384+
385+
test_expect_success 'log --graph with multiple root commits' '
386+
test_commit 8_B1 &&
387+
git checkout --orphan 8_3 && test_commit 8_C &&
388+
389+
check_graph 8_3 8_2 8_1 <<-\EOF
390+
* 8_C
391+
* 8_B1
392+
/
393+
* 8_B
394+
* 8_A1
395+
/
396+
* 8_A
397+
EOF
398+
'
399+
400+
test_expect_success 'log --graph commit from a two parent merge shifted' '
401+
git checkout --orphan 9_1 && test_commit 9_B &&
402+
git checkout --orphan 9_2 && test_commit 9_C &&
403+
git checkout 9_1 &&
404+
git merge 9_2 --allow-unrelated-histories -m 9_M &&
405+
git checkout --orphan 9_3 &&
406+
test_commit 9_A && test_commit 9_A1 && test_commit 9_A2 &&
407+
408+
check_graph 9_3 9_1 <<-\EOF
409+
* 9_A2
410+
* 9_A1
411+
* 9_A
412+
* 9_M
413+
/|
414+
| * 9_C
415+
* 9_B
416+
EOF
417+
'
418+
419+
test_expect_success 'log --graph commit from a three parent merge shifted' '
420+
git checkout --orphan 10_1 && test_commit 10_B &&
421+
git checkout --orphan 10_2 && test_commit 10_C &&
422+
git checkout --orphan 10_3 && test_commit 10_D &&
423+
git checkout 10_1 &&
424+
TREE=$(git write-tree) &&
425+
MERGE=$(git commit-tree $TREE -p 10_1 -p 10_2 -p 10_3 -m 10_M) &&
426+
git reset --hard $MERGE &&
427+
git checkout --orphan 10_4 &&
428+
test_commit 10_A && test_commit 10_A1 && test_commit 10_A2 &&
429+
430+
check_graph 10_4 10_1 <<-\EOF
431+
* 10_A2
432+
* 10_A1
433+
* 10_A
434+
* 10_M
435+
/|\
436+
| | * 10_D
437+
| * 10_C
438+
* 10_B
439+
EOF
440+
'
441+
442+
test_expect_success 'log --graph commit from a four parent merge shifted' '
443+
git checkout --orphan 11_1 && test_commit 11_B &&
444+
git checkout --orphan 11_2 && test_commit 11_C &&
445+
git checkout --orphan 11_3 && test_commit 11_D &&
446+
git checkout --orphan 11_4 && test_commit 11_E &&
447+
git checkout 11_1 &&
448+
TREE=$(git write-tree) &&
449+
MERGE=$(git commit-tree $TREE -p 11_1 -p 11_2 -p 11_3 -p 11_4 -m 11_M) &&
450+
git reset --hard $MERGE &&
451+
git checkout --orphan 11_5 &&
452+
test_commit 11_A && test_commit 11_A1 && test_commit 11_A2 &&
453+
454+
check_graph 11_5 11_1 <<-\EOF
455+
* 11_A2
456+
* 11_A1
457+
* 11_A
458+
*-. 11_M
459+
/|\ \
460+
| | | * 11_E
461+
| | * 11_D
462+
| * 11_C
463+
* 11_B
464+
EOF
465+
'
466+
467+
test_expect_success 'log --graph disconnected three roots cascading' '
468+
git checkout --orphan 12_1 && test_commit 12_D && test_commit 12_D1 &&
469+
git checkout --orphan 12_2 && test_commit 12_C &&
470+
git checkout --orphan 12_3 && test_commit 12_B &&
471+
git checkout --orphan 12_4 && test_commit 12_A &&
472+
473+
check_graph 12_4 12_3 12_2 12_1 <<-\EOF
474+
* 12_A
475+
* 12_B
476+
* 12_C
477+
* 12_D1
478+
_ /
479+
/
480+
/
481+
* 12_D
482+
EOF
483+
'
484+
485+
test_expect_success 'log --graph with excluded parent (not a root)' '
486+
git checkout --orphan 13_1 && test_commit 13_X && test_commit 13_Y &&
487+
git checkout --orphan 13_2 && test_commit 13_O && test_commit 13_A &&
488+
489+
check_graph 13_O..13_A 13_1 <<-\EOF
490+
* 13_A
491+
* 13_Y
492+
/
493+
* 13_X
494+
EOF
495+
'
496+
373497
test_done

0 commit comments

Comments
 (0)