* [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
@ 2026-04-02 21:17 Pablo Sabater
2026-04-02 21:17 ` [GSoC RFC PATCH 1/1] " Pablo Sabater
` (3 more replies)
0 siblings, 4 replies; 18+ messages in thread
From: Pablo Sabater @ 2026-04-02 21:17 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
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, which makes the graph confusing.
This issue was reported by Junio at:
https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
e.g.:
* root-B
* child-A2
* child-A1
* root-A
This happens because the engine prints left to right from the first free
column and the root has no parents thus for the next row, its column
becomes free and the engine fills that gap with the next commit (child-A2)
seeming that root-B and child-A2 are related when they are not.
The actual implementation is very minimal.
This patch makes the roots to be kept alive at least one more row to avoid
that and indent the next commit to the next column and then clean the mapping
letting the indented commit to naturally collapse to the column where the
root was.
e.g.:
* root-B
* child-A2
/
* child-A1
* root-A
This is done by adding a is_placeholder flag to the columns, the root commit
is actually there but marked as a placeholder
e.g.:
* root-B
(B) * child-A2
/
* child-A1
* root-A
(B) would be root-B column with the placeholder flag active.
Then teaching the rendering function to print a padding ' ' when meeting a
placeholder column outputs the second example.
There could also be the case where there are multiple roots
without the patch:
* A root
* B root
* C root
* D1 child
* D root
with the patch, the indentation cascades:
* A root
* B root
* C root
* D1 child
_ /
/
/
* D root
the _ / might look weird but that's how the collapsing rendering does it
for big gaps, this case being from the 4th column to the 0th column.
Another patch could change the collapsing rendering for placeholders ?
I haven't done it to keep it minimal, but a follow up could make it
to be straight '/'. This would make it bigger but easier for the eye to follow.
IMO is not worth it, but opinions are welcome.
The patch also adds tests for different cases like a root preceding multiple
parents merges and the examples above.
There could be some edge cases still so any testing is very welcome.
Pablo Sabater (1):
graph: add indentation for commits preceded by a root
graph.c | 68 ++++++++++++++++--
t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
2 files changed, 198 insertions(+), 6 deletions(-)
base-commit: 256554692df0685b45e60778b08802b720880c50
--
2.43.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [GSoC RFC PATCH 1/1] graph: add indentation for commits preceded by a root
2026-04-02 21:17 [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Pablo Sabater
@ 2026-04-02 21:17 ` Pablo Sabater
2026-04-03 17:55 ` Junio C Hamano
2026-04-03 5:04 ` [GSoC RFC PATCH 0/1] " Junio C Hamano
` (2 subsequent siblings)
3 siblings, 1 reply; 18+ messages in thread
From: Pablo Sabater @ 2026-04-02 21:17 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
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>
---
graph.c | 68 ++++++++++++++++--
t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
2 files changed, 198 insertions(+), 6 deletions(-)
diff --git a/graph.c b/graph.c
index 26f6fbf000..736c4a0a0c 100644
--- a/graph.c
+++ b/graph.c
@@ -60,6 +60,13 @@ struct column {
* index into column_colors.
*/
unsigned short color;
+ /*
+ * A placeholder column keeps the column of the root filled for one
+ * extra row, avoiding a next unrelated commit to be printed in the
+ * same column. Placeholder columns don't propagate to the following
+ * commit.
+ */
+ unsigned is_placeholder:1;
};
enum graph_state {
@@ -563,6 +570,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
i = graph->num_new_columns++;
graph->new_columns[i].commit = commit;
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+ graph->new_columns[i].is_placeholder = 0;
}
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -607,7 +615,7 @@ static void graph_update_columns(struct git_graph *graph)
{
struct commit_list *parent;
int max_new_columns;
- int i, seen_this, is_commit_in_columns;
+ int i, seen_this, is_commit_in_columns, is_root;
/*
* Swap graph->columns with graph->new_columns
@@ -654,6 +662,9 @@ static void graph_update_columns(struct git_graph *graph)
*/
seen_this = 0;
is_commit_in_columns = 1;
+ is_root = graph->num_parents == 0 &&
+ !graph->commit->parents &&
+ !(graph->commit->object.flags & BOUNDARY);
for (i = 0; i <= graph->num_columns; i++) {
struct commit *col_commit;
if (i == graph->num_columns) {
@@ -688,11 +699,40 @@ static void graph_update_columns(struct git_graph *graph)
* least 2, even if it has no interesting parents.
* The current commit always takes up at least 2
* spaces.
+ *
+ * Check for the commit to be a root, no parents
+ * and that it is not a boundary commit. If so, add a
+ * placeholder to keep that column filled for
+ * at least one row.
+ *
+ * Prevents the next commit from being inserted
+ * just below and making the graph confusing.
*/
- if (graph->num_parents == 0)
+ if (is_root) {
+ graph_insert_into_new_columns(graph, graph->commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (graph->num_parents == 0) {
graph->width += 2;
+ }
} else {
- graph_insert_into_new_columns(graph, col_commit, -1);
+ if (graph->columns[i].is_placeholder) {
+ /*
+ * Keep the placeholders if the next commit is
+ * a root also, making the indentation cascade.
+ */
+ if (!seen_this && is_root) {
+ graph_insert_into_new_columns(graph,
+ graph->columns[i].commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (!seen_this) {
+ graph->mapping[graph->width] = -1;
+ graph->width += 2;
+ }
+ } else {
+ graph_insert_into_new_columns(graph, col_commit, -1);
+ }
}
}
@@ -846,7 +886,10 @@ static void graph_output_padding_line(struct git_graph *graph,
* Output a padding row, that leaves all branch lines unchanged
*/
for (i = 0; i < graph->num_new_columns; i++) {
- graph_line_write_column(line, &graph->new_columns[i], '|');
+ if (graph->new_columns[i].is_placeholder)
+ graph_line_write_column(line, &graph->new_columns[i], ' ');
+ else
+ graph_line_write_column(line, &graph->new_columns[i], '|');
graph_line_addch(line, ' ');
}
}
@@ -1058,7 +1101,13 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
graph->mapping[2 * i] < i) {
graph_line_write_column(line, col, '/');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
}
graph_line_addch(line, ' ');
}
@@ -1135,7 +1184,14 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
graph_line_write_column(line, col, '|');
graph_line_addch(line, ' ');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
+
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
if (parent_col)
graph_line_write_column(
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..0333fea95a 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,140 @@ test_expect_success 'log --graph with multiple tips' '
EOF
'
+test_expect_success 'log --graph with root commit' '
+ git checkout --orphan 8_a &&
+ test_commit 8_A &&
+ test_commit 8_A1 &&
+ git checkout --orphan 8_b &&
+ test_commit 8_B &&
+
+ check_graph 8_b 8_a <<-\EOF
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph with multiple root commits' '
+ test_commit 8_B1 &&
+ git checkout --orphan 8_c &&
+ test_commit 8_C &&
+
+ check_graph 8_c 8_b 8_a <<-\EOF
+ * 8_C
+ * 8_B1
+ /
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph commit from a two parent merge shifted' '
+ git checkout --orphan 9_b &&
+ test_commit 9_B &&
+ git checkout --orphan 9_c &&
+ test_commit 9_C &&
+ git checkout 9_b &&
+ git merge 9_c --allow-unrelated-histories -m 9_M &&
+ git checkout --orphan 9_a &&
+ test_commit 9_A &&
+ test_commit 9_A1 &&
+ test_commit 9_A2 &&
+
+ check_graph 9_a 9_b <<-\EOF
+ * 9_A2
+ * 9_A1
+ * 9_A
+ * 9_M
+ /|
+ | * 9_C
+ * 9_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a three parent merge shifted' '
+ git checkout --orphan 10_b &&
+ test_commit 10_B &&
+ git checkout --orphan 10_c &&
+ test_commit 10_C &&
+ git checkout --orphan 10_d &&
+ test_commit 10_D &&
+ git checkout 10_b &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 10_b -p 10_c -p 10_d -m 10_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 10_a &&
+ test_commit 10_A &&
+ test_commit 10_A1 &&
+ test_commit 10_A2 &&
+
+ check_graph 10_a 10_b <<-\EOF
+ * 10_A2
+ * 10_A1
+ * 10_A
+ * 10_M
+ /|\
+ | | * 10_D
+ | * 10_C
+ * 10_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a four parent merge shifted' '
+ git checkout --orphan 11_b &&
+ test_commit 11_B &&
+ git checkout --orphan 11_c &&
+ test_commit 11_C &&
+ git checkout --orphan 11_d &&
+ test_commit 11_D &&
+ git checkout --orphan 11_e &&
+ test_commit 11_E &&
+ git checkout 11_b &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 11_b -p 11_c -p 11_d -p 11_e -m 11_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 11_a &&
+ test_commit 11_A &&
+ test_commit 11_A1 &&
+ test_commit 11_A2 &&
+
+ check_graph 11_a 11_b <<-\EOF
+ * 11_A2
+ * 11_A1
+ * 11_A
+ *-. 11_M
+ /|\ \
+ | | | * 11_E
+ | | * 11_D
+ | * 11_C
+ * 11_B
+ EOF
+'
+
+test_expect_success 'log --graph disconnected three roots cascading' '
+ git checkout --orphan 12_d &&
+ test_commit 12_D &&
+ test_commit 12_D1 &&
+ git checkout --orphan 12_c &&
+ test_commit 12_C &&
+ git checkout --orphan 12_b &&
+ test_commit 12_B &&
+ git checkout --orphan 12_a &&
+ test_commit 12_A &&
+
+ check_graph 12_a 12_b 12_c 12_d <<-\EOF
+ * 12_A
+ * 12_B
+ * 12_C
+ * 12_D1
+ _ /
+ /
+ /
+ * 12_D
+ EOF
+'
+
test_done
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
2026-04-02 21:17 [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Pablo Sabater
2026-04-02 21:17 ` [GSoC RFC PATCH 1/1] " Pablo Sabater
@ 2026-04-03 5:04 ` Junio C Hamano
2026-04-03 8:25 ` Pablo
2026-04-04 9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
2026-05-14 15:15 ` [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Phillip Wood
3 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2026-04-03 5:04 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> This issue was reported by Junio at:
> https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
You are giving me too much credit. I just knew about previous
attempts and the gotchas.
One thing that we may want to make sure your solution gets right is
the issue depicated in two graphs in the footnote of this message:
https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
Stepping back a bit, I think concentrating too much on "is it
root?" is a wrong way to think about the problem. Suppose you
have two histories, e.g. (time flows from left to right; A and X
are roots)
A---B
\
X---Y---Z
and doing "git log --graph --oneline Z" would show A, B, X, Y
and Z.
But in a slightly modified graph:
C
/
O---A---B
\
X---Y---Z
if you do "git log --graph --oneline C..Z", you should see the
same commits listed as above (A, B, X, Y and Z), and most likely
in the same order.
The way we draw A and make sure one raw below A in the same lane is
vacant (to avoid something that is not an ancestor of A steals that
spot) is applicable to both graphs. The reason why we try to keep
one row below A vacant is not because it is a root, but because in
the graph being drawn, none of A's parent will appear. Obviously if
A is root, none of A's parent will appear in the graph, but in the
latter topology where we are drawing C..Z, none of A's parent will
appear not because A is root, but because all the parents of A is
excluded.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
2026-04-03 5:04 ` [GSoC RFC PATCH 0/1] " Junio C Hamano
@ 2026-04-03 8:25 ` Pablo
0 siblings, 0 replies; 18+ messages in thread
From: Pablo @ 2026-04-03 8:25 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
El vie, 3 abr 2026 a las 7:04, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > This issue was reported by Junio at:
> > https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
>
> You are giving me too much credit. I just knew about previous
> attempts and the gotchas.
Should I mention that thread instead ?
>
> One thing that we may want to make sure your solution gets right is
> the issue depicated in two graphs in the footnote of this message:
>
> https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
>
> Stepping back a bit, I think concentrating too much on "is it
> root?" is a wrong way to think about the problem. Suppose you
> have two histories, e.g. (time flows from left to right; A and X
> are roots)
>
> A---B
> \
> X---Y---Z
>
> and doing "git log --graph --oneline Z" would show A, B, X, Y
> and Z.
>
> But in a slightly modified graph:
>
> C
> /
> O---A---B
> \
> X---Y---Z
>
> if you do "git log --graph --oneline C..Z", you should see the
> same commits listed as above (A, B, X, Y and Z), and most likely
> in the same order.
I can't find the issue with the graph above, it would be shown:
A---B
\
X---Y---Z
but we shouldn't want indentation here tho
* Z
|\
| * B
| * A
* Y
* X
B is the parent of A and there is no commit on a third branch that
could try to get below A.
But I do find the issue with focusing on: is a root ? for example with
this graph:
O---A
X---Y
If we O..A Y, it shows A, Y, X but because I only look for roots it
ends up looking like:
* A <- not a root but O is excluded
* Y <- no indentation
* X
Then it's more something like 'seems_root' rather than 'is_root', and
it should look like
* A
* Y
/
* X
This would make on your last graph to have indentation at the right of
Y, below A, even if not needed, it would protect A's spot the row
below, which I think is desirable and what you meant with the
examples.
>
> The way we draw A and make sure one raw below A in the same lane is
> vacant (to avoid something that is not an ancestor of A steals that
> spot) is applicable to both graphs. The reason why we try to keep
> one row below A vacant is not because it is a root, but because in
> the graph being drawn, none of A's parent will appear. Obviously if
> A is root, none of A's parent will appear in the graph, but in the
> latter topology where we are drawing C..Z, none of A's parent will
> appear not because A is root, but because all the parents of A is
> excluded.
>
Thanks for the feedback
Pablo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 1/1] graph: add indentation for commits preceded by a root
2026-04-02 21:17 ` [GSoC RFC PATCH 1/1] " Pablo Sabater
@ 2026-04-03 17:55 ` Junio C Hamano
2026-04-03 18:07 ` Pablo
0 siblings, 1 reply; 18+ messages in thread
From: Junio C Hamano @ 2026-04-03 17:55 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
Pablo Sabater <pabloosabaterr@gmail.com> writes:
> diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> index 28d0779a8c..0333fea95a 100755
> --- a/t/t4215-log-skewed-merges.sh
> +++ b/t/t4215-log-skewed-merges.sh
> @@ -370,4 +370,140 @@ test_expect_success 'log --graph with multiple tips' '
> EOF
> '
>
> +test_expect_success 'log --graph with root commit' '
> + git checkout --orphan 8_a &&
> + test_commit 8_A &&
> + test_commit 8_A1 &&
> + git checkout --orphan 8_b &&
> + test_commit 8_B &&
On case challenged filesystems, you cannot have a commit "8_a" and
"8_A" without being ambiguous. The CI failures from last night are
all from Windows and macOS X.
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 1/1] graph: add indentation for commits preceded by a root
2026-04-03 17:55 ` Junio C Hamano
@ 2026-04-03 18:07 ` Pablo
0 siblings, 0 replies; 18+ messages in thread
From: Pablo @ 2026-04-03 18:07 UTC (permalink / raw)
To: Junio C Hamano
Cc: git, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
El vie, 3 abr 2026 a las 19:55, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
> > index 28d0779a8c..0333fea95a 100755
> > --- a/t/t4215-log-skewed-merges.sh
> > +++ b/t/t4215-log-skewed-merges.sh
> > @@ -370,4 +370,140 @@ test_expect_success 'log --graph with multiple tips' '
> > EOF
> > '
> >
> > +test_expect_success 'log --graph with root commit' '
> > + git checkout --orphan 8_a &&
> > + test_commit 8_A &&
> > + test_commit 8_A1 &&
> > + git checkout --orphan 8_b &&
> > + test_commit 8_B &&
>
> On case challenged filesystems, you cannot have a commit "8_a" and
> "8_A" without being ambiguous. The CI failures from last night are
> all from Windows and macOS X.
>
>
>
Okay I'll send a v2 with the "seems_root" change and fix the names,
hadn't thought about it , I'll make sure that CI passes, sorry.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit
2026-04-02 21:17 [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Pablo Sabater
2026-04-02 21:17 ` [GSoC RFC PATCH 1/1] " Pablo Sabater
2026-04-03 5:04 ` [GSoC RFC PATCH 0/1] " Junio C Hamano
@ 2026-04-04 9:24 ` Pablo Sabater
2026-04-04 9:24 ` [GSoC RFC PATCH v2 1/1] " Pablo Sabater
` (2 more replies)
2026-05-14 15:15 ` [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Phillip Wood
3 siblings, 3 replies; 18+ messages in thread
From: Pablo Sabater @ 2026-04-04 9:24 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
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.
e.g.:
* parentless-B
* child-A2
* child-A1
* parentless-A
This issue has been attempted multiple times:
https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
This happens because the engine prints left to right from the first free
column and these parentless commits for the next row, their column
becomes empty and the engine fills that gap with the next commit (child-A2)
seeming that parentless-B and child-A2 are related when they are not.
The actual implementation is very minimal.
This patch makes the parentless commits to be kept alive at least one more row to avoid
that, indenting the next commit to the next column and then clean the mapping
letting the indented commit to naturally collapse to the column where the
parentless commit was.
e.g.:
* parentless-B
* child-A2
/
* child-A1
* parentless-A
This is done by adding a is_placeholder flag to the columns, the parentless
commit is actually there but marked as a placeholder
e.g.:
* parentless-B
(B) * child-A2
/
* child-A1
* parentless-A
(B) would be parentless-B column with the placeholder flag active.
By teaching the rendering function to print a padding ' ' when meeting a
placeholder column hides them, printing the second example.
There could also be the case where there are multiple parentless commits
without the patch:
* A parentless
* B parentless
* C parentless
* D1 child
* D parentless
with the patch, the indentation cascades:
* A parentless
* B parentless
* C parentless
* D1 child
_ /
/
/
* D parentless
the _ / might look weird but that's how the collapsing rendering does it
for big gaps, this case being from the 4th column to the 0th column.
Another patch could change the collapsing rendering for placeholders?
I haven't done it to keep it minimal, but a follow up could make it
to be straight '/'. This would make it bigger but easier for the eye to follow.
IMO is not worth it, but opinions are welcome.
The patch also adds tests for different cases like a parentless commit
preceding multiple parents merges and the examples above.
There could be some edge cases still so any testing is very welcome.
PSA: the tests are on t4215-log-skewed-merges.sh, which is not very related,
but other graph related tests have +140 tests, and this one has less than
20 and some of them are also not very related and differ in style.
A cleanup patch before this renaming the file and style of the tests is fine?
Changes from v1:
- Changed to parentless commits instead of root commits to make it more generic
- Fixed the branch names to pass CI and fixed tests style.
Pablo Sabater (1):
graph: add indentation for commits preceded by a parentless commit
graph.c | 70 ++++++++++++++++++--
t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
2 files changed, 188 insertions(+), 6 deletions(-)
base-commit: 8de2f1b07a8053d7f1aad70dc1131d6afcf5a28a
--
2.43.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [GSoC RFC PATCH v2 1/1] graph: add indentation for commits preceded by a parentless commit
2026-04-04 9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
@ 2026-04-04 9:24 ` Pablo Sabater
2026-04-10 16:25 ` [GSoC RFC PATCH v2 0/1] " Pablo
2026-04-27 10:28 ` [GSoC PATCH v3 " Pablo Sabater
2 siblings, 0 replies; 18+ messages in thread
From: Pablo Sabater @ 2026-04-04 9:24 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
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>
---
graph.c | 70 ++++++++++++++++++--
t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
2 files changed, 188 insertions(+), 6 deletions(-)
diff --git a/graph.c b/graph.c
index 26f6fbf000..e2b7516651 100644
--- a/graph.c
+++ b/graph.c
@@ -60,6 +60,12 @@ struct column {
* index into column_colors.
*/
unsigned short color;
+ /*
+ * A placeholder column keeps the column of a parentless commit filled
+ * for one extra row, avoiding a next unrelated commit to be printed
+ * in the same column.
+ */
+ unsigned is_placeholder:1;
};
enum graph_state {
@@ -563,6 +569,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
i = graph->num_new_columns++;
graph->new_columns[i].commit = commit;
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+ graph->new_columns[i].is_placeholder = 0;
}
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -607,7 +614,7 @@ static void graph_update_columns(struct git_graph *graph)
{
struct commit_list *parent;
int max_new_columns;
- int i, seen_this, is_commit_in_columns;
+ int i, seen_this, is_commit_in_columns, seems_root;
/*
* Swap graph->columns with graph->new_columns
@@ -654,6 +661,12 @@ static void graph_update_columns(struct git_graph *graph)
*/
seen_this = 0;
is_commit_in_columns = 1;
+ /*
+ * num_parents == 0 means that there are no parents flagged as
+ * interesting to being shown.
+ */
+ seems_root = graph->num_parents == 0 &&
+ !(graph->commit->object.flags & BOUNDARY);
for (i = 0; i <= graph->num_columns; i++) {
struct commit *col_commit;
if (i == graph->num_columns) {
@@ -688,11 +701,40 @@ static void graph_update_columns(struct git_graph *graph)
* least 2, even if it has no interesting parents.
* The current commit always takes up at least 2
* spaces.
+ *
+ * Check for the commit to seem like a root, no parents
+ * rendered and that it is not a boundary commit. If so,
+ * add a placeholder to keep that column filled for
+ * at least one row.
+ *
+ * Prevents the next commit from being inserted
+ * just below and making the graph confusing.
*/
- if (graph->num_parents == 0)
+ if (seems_root) {
+ graph_insert_into_new_columns(graph, graph->commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (graph->num_parents == 0) {
graph->width += 2;
+ }
} else {
- graph_insert_into_new_columns(graph, col_commit, -1);
+ if (graph->columns[i].is_placeholder) {
+ /*
+ * Keep the placeholders if the next commit is
+ * parentless also, making the indentation cascade.
+ */
+ if (!seen_this && seems_root) {
+ graph_insert_into_new_columns(graph,
+ graph->columns[i].commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (!seen_this) {
+ graph->mapping[graph->width] = -1;
+ graph->width += 2;
+ }
+ } else {
+ graph_insert_into_new_columns(graph, col_commit, -1);
+ }
}
}
@@ -846,7 +888,10 @@ static void graph_output_padding_line(struct git_graph *graph,
* Output a padding row, that leaves all branch lines unchanged
*/
for (i = 0; i < graph->num_new_columns; i++) {
- graph_line_write_column(line, &graph->new_columns[i], '|');
+ if (graph->new_columns[i].is_placeholder)
+ graph_line_write_column(line, &graph->new_columns[i], ' ');
+ else
+ graph_line_write_column(line, &graph->new_columns[i], '|');
graph_line_addch(line, ' ');
}
}
@@ -1058,7 +1103,13 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
graph->mapping[2 * i] < i) {
graph_line_write_column(line, col, '/');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
}
graph_line_addch(line, ' ');
}
@@ -1135,7 +1186,14 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
graph_line_write_column(line, col, '|');
graph_line_addch(line, ' ');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
+
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
if (parent_col)
graph_line_write_column(
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..0f6f95a6b5 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,128 @@ test_expect_success 'log --graph with multiple tips' '
EOF
'
+test_expect_success 'log --graph with root commit' '
+ git checkout --orphan 8_1 && test_commit 8_A && test_commit 8_A1 &&
+ git checkout --orphan 8_2 && test_commit 8_B &&
+
+ check_graph 8_2 8_1 <<-\EOF
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph with multiple root commits' '
+ test_commit 8_B1 &&
+ git checkout --orphan 8_3 && test_commit 8_C &&
+
+ check_graph 8_3 8_2 8_1 <<-\EOF
+ * 8_C
+ * 8_B1
+ /
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph commit from a two parent merge shifted' '
+ git checkout --orphan 9_1 && test_commit 9_B &&
+ git checkout --orphan 9_2 && test_commit 9_C &&
+ git checkout 9_1 &&
+ git merge 9_2 --allow-unrelated-histories -m 9_M &&
+ git checkout --orphan 9_3 &&
+ test_commit 9_A && test_commit 9_A1 && test_commit 9_A2 &&
+
+ check_graph 9_3 9_1 <<-\EOF
+ * 9_A2
+ * 9_A1
+ * 9_A
+ * 9_M
+ /|
+ | * 9_C
+ * 9_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a three parent merge shifted' '
+ git checkout --orphan 10_1 && test_commit 10_B &&
+ git checkout --orphan 10_2 && test_commit 10_C &&
+ git checkout --orphan 10_3 && test_commit 10_D &&
+ git checkout 10_1 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 10_1 -p 10_2 -p 10_3 -m 10_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 10_4 &&
+ test_commit 10_A && test_commit 10_A1 && test_commit 10_A2 &&
+
+ check_graph 10_4 10_1 <<-\EOF
+ * 10_A2
+ * 10_A1
+ * 10_A
+ * 10_M
+ /|\
+ | | * 10_D
+ | * 10_C
+ * 10_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a four parent merge shifted' '
+ git checkout --orphan 11_1 && test_commit 11_B &&
+ git checkout --orphan 11_2 && test_commit 11_C &&
+ git checkout --orphan 11_3 && test_commit 11_D &&
+ git checkout --orphan 11_4 && test_commit 11_E &&
+ git checkout 11_1 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 11_1 -p 11_2 -p 11_3 -p 11_4 -m 11_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 11_5 &&
+ test_commit 11_A && test_commit 11_A1 && test_commit 11_A2 &&
+
+ check_graph 11_5 11_1 <<-\EOF
+ * 11_A2
+ * 11_A1
+ * 11_A
+ *-. 11_M
+ /|\ \
+ | | | * 11_E
+ | | * 11_D
+ | * 11_C
+ * 11_B
+ EOF
+'
+
+test_expect_success 'log --graph disconnected three roots cascading' '
+ git checkout --orphan 12_1 && test_commit 12_D && test_commit 12_D1 &&
+ git checkout --orphan 12_2 && test_commit 12_C &&
+ git checkout --orphan 12_3 && test_commit 12_B &&
+ git checkout --orphan 12_4 && test_commit 12_A &&
+
+ check_graph 12_4 12_3 12_2 12_1 <<-\EOF
+ * 12_A
+ * 12_B
+ * 12_C
+ * 12_D1
+ _ /
+ /
+ /
+ * 12_D
+ EOF
+'
+
+test_expect_success 'log --graph with excluded parent (not a root)' '
+ git checkout --orphan 13_1 && test_commit 13_X && test_commit 13_Y &&
+ git checkout --orphan 13_2 && test_commit 13_O && test_commit 13_A &&
+
+ check_graph 13_O..13_A 13_1 <<-\EOF
+ * 13_A
+ * 13_Y
+ /
+ * 13_X
+ EOF
+'
+
test_done
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit
2026-04-04 9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
2026-04-04 9:24 ` [GSoC RFC PATCH v2 1/1] " Pablo Sabater
@ 2026-04-10 16:25 ` Pablo
2026-04-10 16:54 ` Junio C Hamano
2026-04-27 10:28 ` [GSoC PATCH v3 " Pablo Sabater
2 siblings, 1 reply; 18+ messages in thread
From: Pablo @ 2026-04-10 16:25 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
El sáb, 4 abr 2026 a las 11:24, Pablo Sabater
(<pabloosabaterr@gmail.com>) escribió:
>
> 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.
>
> e.g.:
>
> * parentless-B
> * child-A2
> * child-A1
> * parentless-A
>
> This issue has been attempted multiple times:
> https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
>
> This happens because the engine prints left to right from the first free
> column and these parentless commits for the next row, their column
> becomes empty and the engine fills that gap with the next commit (child-A2)
> seeming that parentless-B and child-A2 are related when they are not.
>
> The actual implementation is very minimal.
> This patch makes the parentless commits to be kept alive at least one more row to avoid
> that, indenting the next commit to the next column and then clean the mapping
> letting the indented commit to naturally collapse to the column where the
> parentless commit was.
>
> e.g.:
>
> * parentless-B
> * child-A2
> /
> * child-A1
> * parentless-A
>
> This is done by adding a is_placeholder flag to the columns, the parentless
> commit is actually there but marked as a placeholder
>
> e.g.:
>
> * parentless-B
> (B) * child-A2
> /
> * child-A1
> * parentless-A
>
> (B) would be parentless-B column with the placeholder flag active.
>
> By teaching the rendering function to print a padding ' ' when meeting a
> placeholder column hides them, printing the second example.
>
> There could also be the case where there are multiple parentless commits
>
> without the patch:
>
> * A parentless
> * B parentless
> * C parentless
> * D1 child
> * D parentless
>
> with the patch, the indentation cascades:
>
> * A parentless
> * B parentless
> * C parentless
> * D1 child
> _ /
> /
> /
> * D parentless
>
> the _ / might look weird but that's how the collapsing rendering does it
> for big gaps, this case being from the 4th column to the 0th column.
>
> Another patch could change the collapsing rendering for placeholders?
> I haven't done it to keep it minimal, but a follow up could make it
> to be straight '/'. This would make it bigger but easier for the eye to follow.
> IMO is not worth it, but opinions are welcome.
>
> The patch also adds tests for different cases like a parentless commit
> preceding multiple parents merges and the examples above.
>
> There could be some edge cases still so any testing is very welcome.
>
> PSA: the tests are on t4215-log-skewed-merges.sh, which is not very related,
> but other graph related tests have +140 tests, and this one has less than
> 20 and some of them are also not very related and differ in style.
> A cleanup patch before this renaming the file and style of the tests is fine?
>
> Changes from v1:
>
> - Changed to parentless commits instead of root commits to make it more generic
> - Fixed the branch names to pass CI and fixed tests style.
>
> Pablo Sabater (1):
> graph: add indentation for commits preceded by a parentless commit
>
> graph.c | 70 ++++++++++++++++++--
> t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
> 2 files changed, 188 insertions(+), 6 deletions(-)
>
>
> base-commit: 8de2f1b07a8053d7f1aad70dc1131d6afcf5a28a
> --
> 2.43.0
>
Hi,
I'm sending this because I think it has fallen through.
Sorry about the ping,
Pablo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit
2026-04-10 16:25 ` [GSoC RFC PATCH v2 0/1] " Pablo
@ 2026-04-10 16:54 ` Junio C Hamano
0 siblings, 0 replies; 18+ messages in thread
From: Junio C Hamano @ 2026-04-10 16:54 UTC (permalink / raw)
To: Pablo
Cc: git, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
Pablo <pabloosabaterr@gmail.com> writes:
> El sáb, 4 abr 2026 a las 11:24, Pablo Sabater
> (<pabloosabaterr@gmail.com>) escribió:
>>
>> When having a history with multiple root commits or commits
>> that act like roots (they have excluded parents), let's call
> ...
> Hi,
> I'm sending this because I think it has fallen through.
> Sorry about the ping,
> Pablo
Pinging is good than no pinging, so no need to say sorry.
I think this topic (the one on April 04) has been on the "What's
cooking" report since issue #02 of this month, waiting for comments
and responses to them. I haven't seen problems in it but that may
be because I do not view commits near the root commit very often.
Thanks.
^ permalink raw reply [flat|nested] 18+ messages in thread
* [GSoC PATCH v3 0/1] graph: add indentation for commits preceded by a parentless commit
2026-04-04 9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
2026-04-04 9:24 ` [GSoC RFC PATCH v2 1/1] " Pablo Sabater
2026-04-10 16:25 ` [GSoC RFC PATCH v2 0/1] " Pablo
@ 2026-04-27 10:28 ` Pablo Sabater
2026-04-27 10:28 ` [GSoC PATCH v3 1/1] " Pablo Sabater
2026-04-27 10:35 ` [GSoC PATCH v3 0/1] " Pablo
2 siblings, 2 replies; 18+ messages in thread
From: Pablo Sabater @ 2026-04-27 10:28 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
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:
* parentless A
* child B
* parentless B
This issue has been attempted multiple times:
https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
This happens because the engine prints left to right from the first free
column and their column of these parentless commits for the next row
becomes empty and the engine fills that gap with the next commit (child B)
seeming that parentless A and child B are related when they are not.
The actual implementation is very minimal.
This patch makes the parentless commits to be kept alive at least one more row
to avoid that, indenting the next commit to the next column and then clean
the mapping letting the indented commit to naturally collapse to the column
where the parentless commit was:
* parentless A
* child B
/
* parentless B
This is done by adding a is_placeholder flag to the columns, the parentless
commit is actually there but marked as a placeholder:
* parentless A
(A) * child B
/
* parentless B
(A) would be "parentless A" column with the placeholder flag active.
By teaching the rendering function to print a padding ' ' when meeting a
placeholder column hides them, printing the second example.
There could also be the case where there are multiple parentless commits
without the patch:
* parentless A
* parentless B
* parentless C
* child D
* parentless D
with the patch, the indentation cascades:
* parentless A
* parentless B
* parentless C
* child D
_ /
/
/
* parentless D
the _ / might look weird but that's how the collapsing rendering does it
for big gaps, this case being from the 4th column to the 0th column.
A follow-up could change the collapsing rendering for placeholders?
I haven't done it to keep it minimal, but a follow up could make it
to be straight '/'. This would make it bigger but easier for the eye to follow.
Is not worth it IMO, but opinions are welcome.
The patch also adds tests for different cases like a parentless commit
preceding multiple parents merges and the examples above.
PSA: the tests are on t4215-log-skewed-merges.sh, which is not very related,
but other graph related tests have +140 tests, and this one has less than
20 and some of them are also not very related and differ in style.
A cleanup patch before this renaming the file and style of the tests is fine?
Changes from v2:
- Removed trailing whitespace.
- Added more comments to make it more clear an reviewable.
- Changed is_root to is_parentless to follow the name at the cover letter and
commit.
- simplified cover letter and commit ascii graphs.
Pablo Sabater (1):
graph: add indentation for commits preceded by a parentless commit
graph.c | 115 ++++++++++++++++++++++++++++++--
t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
2 files changed, 233 insertions(+), 6 deletions(-)
base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
--
2.43.0
^ permalink raw reply [flat|nested] 18+ messages in thread
* [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
2026-04-27 10:28 ` [GSoC PATCH v3 " Pablo Sabater
@ 2026-04-27 10:28 ` Pablo Sabater
2026-05-13 23:02 ` Jeff King
2026-04-27 10:35 ` [GSoC PATCH v3 0/1] " Pablo
1 sibling, 1 reply; 18+ messages in thread
From: Pablo Sabater @ 2026-04-27 10:28 UTC (permalink / raw)
To: git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519, Pablo Sabater
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 A
* child B
* parentless B
after this patch:
* parentless A
* child B
/
* parentless B
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
graph.c | 115 ++++++++++++++++++++++++++++++--
t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
2 files changed, 233 insertions(+), 6 deletions(-)
diff --git a/graph.c b/graph.c
index 26f6fbf000..97292df998 100644
--- a/graph.c
+++ b/graph.c
@@ -60,6 +60,12 @@ struct column {
* index into column_colors.
*/
unsigned short color;
+ /*
+ * A placeholder column keeps the column of a parentless commit filled
+ * for one extra row, avoiding a next unrelated commit to be printed
+ * in the same column.
+ */
+ unsigned is_placeholder:1;
};
enum graph_state {
@@ -563,6 +569,7 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
i = graph->num_new_columns++;
graph->new_columns[i].commit = commit;
graph->new_columns[i].color = graph_find_commit_color(graph, commit);
+ graph->new_columns[i].is_placeholder = 0;
}
if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -607,7 +614,7 @@ static void graph_update_columns(struct git_graph *graph)
{
struct commit_list *parent;
int max_new_columns;
- int i, seen_this, is_commit_in_columns;
+ int i, seen_this, is_commit_in_columns, is_parentless;
/*
* Swap graph->columns with graph->new_columns
@@ -654,6 +661,26 @@ static void graph_update_columns(struct git_graph *graph)
*/
seen_this = 0;
is_commit_in_columns = 1;
+ /*
+ * A commit is "parentless" (is a visual root that starts a new column)
+ * only if has no visible parents AND it's not a boundary commit.
+ *
+ * Boundary commits also have no visible parents, but they are
+ * NOT a visual root:
+ *
+ * 1. A boundary only appears in the output because an included commit
+ * is its child. Children are always above, and the renderer draws an
+ * edge down to the boundary from that child. Rather than starting
+ * a column like a visual root would do, it "inherits" its child
+ * column.
+ *
+ * 2. Included commit CAN'T appear below a boundary. Boundaries are
+ * ancestors of the exclusion point; if an included commit were an
+ * ancestor of the boundary it would be excluded and not rendered.
+ * Boundaries therefore always sink to the bottom.
+ */
+ is_parentless = graph->num_parents == 0 &&
+ !(graph->commit->object.flags & BOUNDARY);
for (i = 0; i <= graph->num_columns; i++) {
struct commit *col_commit;
if (i == graph->num_columns) {
@@ -688,11 +715,46 @@ static void graph_update_columns(struct git_graph *graph)
* least 2, even if it has no interesting parents.
* The current commit always takes up at least 2
* spaces.
+ *
+ * Check for the commit to seem like a root, no parents
+ * rendered and that it is not a boundary commit. If so,
+ * add a placeholder to keep that column filled for
+ * at least one row.
+ *
+ * Prevents the next commit from being inserted
+ * just below and making the graph confusing.
*/
- if (graph->num_parents == 0)
+ if (is_parentless) {
+ graph_insert_into_new_columns(graph, graph->commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (graph->num_parents == 0) {
graph->width += 2;
+ }
} else {
- graph_insert_into_new_columns(graph, col_commit, -1);
+ if (graph->columns[i].is_placeholder) {
+ /*
+ * Keep the placeholders if the next commit is
+ * parentless also, making the indentation cascade.
+ */
+ if (!seen_this && is_parentless) {
+ graph_insert_into_new_columns(graph,
+ graph->columns[i].commit, i);
+ graph->new_columns[graph->num_new_columns - 1]
+ .is_placeholder = 1;
+ } else if (!seen_this) {
+ graph->mapping[graph->width] = -1;
+ graph->width += 2;
+ }
+ /*
+ * seen_this && is_placeholder means that this
+ * line is the one after the indented one, the
+ * placeholder is no longer needed, gets
+ * dropped and the columns collapses naturally.
+ */
+ } else {
+ graph_insert_into_new_columns(graph, col_commit, -1);
+ }
}
}
@@ -846,7 +908,10 @@ static void graph_output_padding_line(struct git_graph *graph,
* Output a padding row, that leaves all branch lines unchanged
*/
for (i = 0; i < graph->num_new_columns; i++) {
- graph_line_write_column(line, &graph->new_columns[i], '|');
+ if (graph->new_columns[i].is_placeholder)
+ graph_line_write_column(line, &graph->new_columns[i], ' ');
+ else
+ graph_line_write_column(line, &graph->new_columns[i], '|');
graph_line_addch(line, ' ');
}
}
@@ -1058,7 +1123,34 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
graph->mapping[2 * i] < i) {
graph_line_write_column(line, col, '/');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ /*
+ * When the indented commit is a merge commit,
+ * the placeholder column adds unwanted padding
+ * between the commit and its subject.
+ *
+ * * parentless commit
+ * * merge commit
+ * /|
+ * | * parent A
+ * * parent B
+ * ^^ unwanted padding
+ *
+ * Once the current commit has been seen, don't
+ * let placeholder columns to be rendered:
+ *
+ * * parentless commit
+ * * merge commit
+ * /|
+ * | * parent A
+ * * parent B
+ */
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
}
graph_line_addch(line, ' ');
}
@@ -1135,7 +1227,18 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
graph_line_write_column(line, col, '|');
graph_line_addch(line, ' ');
} else {
- graph_line_write_column(line, col, '|');
+ if (col->is_placeholder) {
+ /*
+ * Same placeholder handling as in
+ * graph_output_commit_line().
+ */
+ if (seen_this)
+ continue;
+ graph_line_write_column(line, col, ' ');
+ } else {
+ graph_line_write_column(line, col, '|');
+ }
+
if (graph->merge_layout != 0 || i != graph->commit_index - 1) {
if (parent_col)
graph_line_write_column(
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 28d0779a8c..0f6f95a6b5 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -370,4 +370,128 @@ test_expect_success 'log --graph with multiple tips' '
EOF
'
+test_expect_success 'log --graph with root commit' '
+ git checkout --orphan 8_1 && test_commit 8_A && test_commit 8_A1 &&
+ git checkout --orphan 8_2 && test_commit 8_B &&
+
+ check_graph 8_2 8_1 <<-\EOF
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph with multiple root commits' '
+ test_commit 8_B1 &&
+ git checkout --orphan 8_3 && test_commit 8_C &&
+
+ check_graph 8_3 8_2 8_1 <<-\EOF
+ * 8_C
+ * 8_B1
+ /
+ * 8_B
+ * 8_A1
+ /
+ * 8_A
+ EOF
+'
+
+test_expect_success 'log --graph commit from a two parent merge shifted' '
+ git checkout --orphan 9_1 && test_commit 9_B &&
+ git checkout --orphan 9_2 && test_commit 9_C &&
+ git checkout 9_1 &&
+ git merge 9_2 --allow-unrelated-histories -m 9_M &&
+ git checkout --orphan 9_3 &&
+ test_commit 9_A && test_commit 9_A1 && test_commit 9_A2 &&
+
+ check_graph 9_3 9_1 <<-\EOF
+ * 9_A2
+ * 9_A1
+ * 9_A
+ * 9_M
+ /|
+ | * 9_C
+ * 9_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a three parent merge shifted' '
+ git checkout --orphan 10_1 && test_commit 10_B &&
+ git checkout --orphan 10_2 && test_commit 10_C &&
+ git checkout --orphan 10_3 && test_commit 10_D &&
+ git checkout 10_1 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 10_1 -p 10_2 -p 10_3 -m 10_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 10_4 &&
+ test_commit 10_A && test_commit 10_A1 && test_commit 10_A2 &&
+
+ check_graph 10_4 10_1 <<-\EOF
+ * 10_A2
+ * 10_A1
+ * 10_A
+ * 10_M
+ /|\
+ | | * 10_D
+ | * 10_C
+ * 10_B
+ EOF
+'
+
+test_expect_success 'log --graph commit from a four parent merge shifted' '
+ git checkout --orphan 11_1 && test_commit 11_B &&
+ git checkout --orphan 11_2 && test_commit 11_C &&
+ git checkout --orphan 11_3 && test_commit 11_D &&
+ git checkout --orphan 11_4 && test_commit 11_E &&
+ git checkout 11_1 &&
+ TREE=$(git write-tree) &&
+ MERGE=$(git commit-tree $TREE -p 11_1 -p 11_2 -p 11_3 -p 11_4 -m 11_M) &&
+ git reset --hard $MERGE &&
+ git checkout --orphan 11_5 &&
+ test_commit 11_A && test_commit 11_A1 && test_commit 11_A2 &&
+
+ check_graph 11_5 11_1 <<-\EOF
+ * 11_A2
+ * 11_A1
+ * 11_A
+ *-. 11_M
+ /|\ \
+ | | | * 11_E
+ | | * 11_D
+ | * 11_C
+ * 11_B
+ EOF
+'
+
+test_expect_success 'log --graph disconnected three roots cascading' '
+ git checkout --orphan 12_1 && test_commit 12_D && test_commit 12_D1 &&
+ git checkout --orphan 12_2 && test_commit 12_C &&
+ git checkout --orphan 12_3 && test_commit 12_B &&
+ git checkout --orphan 12_4 && test_commit 12_A &&
+
+ check_graph 12_4 12_3 12_2 12_1 <<-\EOF
+ * 12_A
+ * 12_B
+ * 12_C
+ * 12_D1
+ _ /
+ /
+ /
+ * 12_D
+ EOF
+'
+
+test_expect_success 'log --graph with excluded parent (not a root)' '
+ git checkout --orphan 13_1 && test_commit 13_X && test_commit 13_Y &&
+ git checkout --orphan 13_2 && test_commit 13_O && test_commit 13_A &&
+
+ check_graph 13_O..13_A 13_1 <<-\EOF
+ * 13_A
+ * 13_Y
+ /
+ * 13_X
+ EOF
+'
+
test_done
--
2.43.0
^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [GSoC PATCH v3 0/1] graph: add indentation for commits preceded by a parentless commit
2026-04-27 10:28 ` [GSoC PATCH v3 " Pablo Sabater
2026-04-27 10:28 ` [GSoC PATCH v3 1/1] " Pablo Sabater
@ 2026-04-27 10:35 ` Pablo
1 sibling, 0 replies; 18+ messages in thread
From: Pablo @ 2026-04-27 10:35 UTC (permalink / raw)
To: git
Cc: Junio C Hamano, Christian Couder, karthik nayak, jltobler,
Ayush Chandekar, Siddharth Asthana, Chandra Pratap
El lun, 27 abr 2026 a las 12:28, Pablo Sabater
(<pabloosabaterr@gmail.com>) escribió:
>
> Pablo Sabater (1):
> graph: add indentation for commits preceded by a parentless commit
>
> graph.c | 115 ++++++++++++++++++++++++++++++--
> t/t4215-log-skewed-merges.sh | 124 +++++++++++++++++++++++++++++++++++
> 2 files changed, 233 insertions(+), 6 deletions(-)
>
>
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0
Hi!
This patch seems to have a problem to get reviewed, I improved the comments
at the code and simplified the example graphs at the cover letter and
patch to try
to make it easier to review.
Let me know if any clarification is needed,
--
Pablo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
2026-04-27 10:28 ` [GSoC PATCH v3 1/1] " Pablo Sabater
@ 2026-05-13 23:02 ` Jeff King
2026-05-14 10:19 ` Pablo Sabater
0 siblings, 1 reply; 18+ messages in thread
From: Jeff King @ 2026-05-13 23:02 UTC (permalink / raw)
To: Pablo Sabater
Cc: git, gitster, christian.couder, karthik.188, jltobler,
ayu.chandekar, siddharthasthana31, chandrapratap3519
On Mon, Apr 27, 2026 at 12:28:38PM +0200, Pablo Sabater wrote:
> @@ -1135,7 +1227,18 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
> graph_line_write_column(line, col, '|');
> graph_line_addch(line, ' ');
> } else {
> - graph_line_write_column(line, col, '|');
> + if (col->is_placeholder) {
> + /*
> + * Same placeholder handling as in
> + * graph_output_commit_line().
> + */
> + if (seen_this)
> + continue;
> + graph_line_write_column(line, col, ' ');
> + } else {
> + graph_line_write_column(line, col, '|');
> + }
I haven't looked closely at the patch, but Coverity complained that
the "if (seen_this)" check here is dead code, because this whole else
block follows:
} else if (seen_this) {
if (graph->edges_added > 0)
graph_line_write_column(line, col, '\\');
else
graph_line_write_column(line, col, '|');
graph_line_addch(line, ' ');
} else {
...the code above...
I don't know if that just means the continue here is redundant and can
be removed, or if it's a sign of a larger logic error.
-Peff
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit
2026-05-13 23:02 ` Jeff King
@ 2026-05-14 10:19 ` Pablo Sabater
0 siblings, 0 replies; 18+ messages in thread
From: Pablo Sabater @ 2026-05-14 10:19 UTC (permalink / raw)
To: Jeff King
Cc: git, gitster, christian.couder, karthik.188, jltobler,
ayu.chandekar, siddharthasthana31, chandrapratap3519
El jue, 14 may 2026 a las 1:02, Jeff King (<peff@peff.net>) escribió:
>
> On Mon, Apr 27, 2026 at 12:28:38PM +0200, Pablo Sabater wrote:
>
> > @@ -1135,7 +1227,18 @@ static void graph_output_post_merge_line(struct git_graph *graph, struct graph_l
> > graph_line_write_column(line, col, '|');
> > graph_line_addch(line, ' ');
> > } else {
> > - graph_line_write_column(line, col, '|');
> > + if (col->is_placeholder) {
> > + /*
> > + * Same placeholder handling as in
> > + * graph_output_commit_line().
> > + */
> > + if (seen_this)
> > + continue;
> > + graph_line_write_column(line, col, ' ');
> > + } else {
> > + graph_line_write_column(line, col, '|');
> > + }
>
> I haven't looked closely at the patch, but Coverity complained that
> the "if (seen_this)" check here is dead code, because this whole else
> block follows:
>
> } else if (seen_this) {
> if (graph->edges_added > 0)
> graph_line_write_column(line, col, '\\');
> else
> graph_line_write_column(line, col, '|');
> graph_line_addch(line, ' ');
> } else {
> ...the code above...
>
> I don't know if that just means the continue here is redundant and can
> be removed, or if it's a sign of a larger logic error.
>
> -Peff
It is dead code. The behaviour for placeholder at
"graph_output_commit_line()" and "graph_output_post_merge_line()" is
the same, if it's a placeholder print a padding instead of an edge,
but I didn't give it a second thought, graph_output_commit_line() can
have a placeholder at its right (that's why it needs the continue to
avoid extra padding) but post merge can't and as it is dead code I
didn't notice.
I'll drop the dead code.
Thanks,
--
Pablo
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
2026-04-02 21:17 [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Pablo Sabater
` (2 preceding siblings ...)
2026-04-04 9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
@ 2026-05-14 15:15 ` Phillip Wood
2026-05-14 17:45 ` Pablo Sabater
3 siblings, 1 reply; 18+ messages in thread
From: Phillip Wood @ 2026-05-14 15:15 UTC (permalink / raw)
To: Pablo Sabater, git
Cc: gitster, christian.couder, karthik.188, jltobler, ayu.chandekar,
siddharthasthana31, chandrapratap3519
Hi Pablo
On 02/04/2026 22:17, Pablo Sabater wrote:
> 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, which makes the graph confusing.
>
> This issue was reported by Junio at:
> https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
>
> e.g.:
>
> * root-B
> * child-A2
> * child-A1
> * root-A
>
> [...]
>
> * root-B
> * child-A2
> /
> * child-A1
> * root-A
I'm rather late to the party here, but personally I find the indentation
a bit confusing, it would be clearer to me if we had a blank line after
a root commit
* root-B
* child-A2
* child-A1
* root-A
It takes the same amount of vertical space but keeps the children of
root-A together.
Thanks
Phillip
> This is done by adding a is_placeholder flag to the columns, the root commit
> is actually there but marked as a placeholder
>
> e.g.:
>
> * root-B
> (B) * child-A2
> /
> * child-A1
> * root-A
>
> (B) would be root-B column with the placeholder flag active.
>
> Then teaching the rendering function to print a padding ' ' when meeting a
> placeholder column outputs the second example.
>
> There could also be the case where there are multiple roots
>
> without the patch:
>
> * A root
> * B root
> * C root
> * D1 child
> * D root
>
> with the patch, the indentation cascades:
>
> * A root
> * B root
> * C root
> * D1 child
> _ /
> /
> /
> * D root
>
> the _ / might look weird but that's how the collapsing rendering does it
> for big gaps, this case being from the 4th column to the 0th column.
> Another patch could change the collapsing rendering for placeholders ?
> I haven't done it to keep it minimal, but a follow up could make it
> to be straight '/'. This would make it bigger but easier for the eye to follow.
> IMO is not worth it, but opinions are welcome.
>
> The patch also adds tests for different cases like a root preceding multiple
> parents merges and the examples above.
>
> There could be some edge cases still so any testing is very welcome.
>
> Pablo Sabater (1):
> graph: add indentation for commits preceded by a root
>
> graph.c | 68 ++++++++++++++++--
> t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
> 2 files changed, 198 insertions(+), 6 deletions(-)
>
>
> base-commit: 256554692df0685b45e60778b08802b720880c50
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
2026-05-14 15:15 ` [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Phillip Wood
@ 2026-05-14 17:45 ` Pablo Sabater
2026-05-15 9:33 ` Phillip Wood
0 siblings, 1 reply; 18+ messages in thread
From: Pablo Sabater @ 2026-05-14 17:45 UTC (permalink / raw)
To: phillip.wood
Cc: git, gitster, christian.couder, karthik.188, jltobler,
ayu.chandekar, siddharthasthana31, chandrapratap3519
El jue, 14 may 2026 a las 17:15, Phillip Wood
(<phillip.wood123@gmail.com>) escribió:
>
> Hi Pablo
>
> On 02/04/2026 22:17, Pablo Sabater wrote:
> > 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, which makes the graph confusing.
> >
> > This issue was reported by Junio at:
> > https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
> >
> > e.g.:
> >
> > * root-B
> > * child-A2
> > * child-A1
> > * root-A
> >
> > [...]
> >
> > * root-B
> > * child-A2
> > /
> > * child-A1
> > * root-A
>
> I'm rather late to the party here, but personally I find the indentation
> a bit confusing, it would be clearer to me if we had a blank line after
> a root commit
Hi,
>
> * root-B
>
> * child-A2
> * child-A1
> * root-A
>
> It takes the same amount of vertical space but keeps the children of
> root-A together.
I have mixed feelings about which approach to choose.
The idea of a blank line was thought at
https://lore.kernel.org/git/xmqq8s8vvw9m.fsf@gitster.c.googlers.com/
but Junio argued against it for having an extra row because the
indentation he proposed didn't collapse, however I find indentation +
no collapse the most confusing one.
I'd say that I'm fine with both approaches, blank line or indentation
+ collapse.
> > without the patch:
> >
> > * A root
> > * B root
> > * C root
> > * D1 child
> > * D root
> >
> > with the patch, the indentation cascades:
> >
> > * A root
> > * B root
> > * C root
> > * D1 child
> > _ /
> > /
> > /
> > * D root
* A root
* B root
* C root
* D1 child
* D root
Here I think a blank line looks worse, too much space for just 5
commits and becomes one extra line which if this were like up to 7 or
more parentless commits one after the other would be more noticeable.
But there are cases that blank line might be better:
* 10_A2
* 10_A1
* 10_A
* 10_M
/|\
| | * 10_D
| * 10_C
* 10_B
Feels like a shower of commits instead of an indented merge.
Pro to the blank line, the parentless check is the same and it's just
printing a '\n' at the right spot, while indent i'm mimicking like if
there was a commit there.
Anyways, I think in the majority of the cases the indentation +
collapsing looks better.
Sorry for the brief reply, I'm busy today.
Regards,
--
Pablo
>
> Thanks
>
> Phillip
>
> > This is done by adding a is_placeholder flag to the columns, the root commit
> > is actually there but marked as a placeholder
> >
> > e.g.:
> >
> > * root-B
> > (B) * child-A2
> > /
> > * child-A1
> > * root-A
> >
> > (B) would be root-B column with the placeholder flag active.
> >
> > Then teaching the rendering function to print a padding ' ' when meeting a
> > placeholder column outputs the second example.
> >
> > There could also be the case where there are multiple roots
> >
> > without the patch:
> >
> > * A root
> > * B root
> > * C root
> > * D1 child
> > * D root
> >
> > with the patch, the indentation cascades:
> >
> > * A root
> > * B root
> > * C root
> > * D1 child
> > _ /
> > /
> > /
> > * D root
> >
> > the _ / might look weird but that's how the collapsing rendering does it
> > for big gaps, this case being from the 4th column to the 0th column.
> > Another patch could change the collapsing rendering for placeholders ?
> > I haven't done it to keep it minimal, but a follow up could make it
> > to be straight '/'. This would make it bigger but easier for the eye to follow.
> > IMO is not worth it, but opinions are welcome.
> >
> > The patch also adds tests for different cases like a root preceding multiple
> > parents merges and the examples above.
> >
> > There could be some edge cases still so any testing is very welcome.
> >
> > Pablo Sabater (1):
> > graph: add indentation for commits preceded by a root
> >
> > graph.c | 68 ++++++++++++++++--
> > t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
> > 2 files changed, 198 insertions(+), 6 deletions(-)
> >
> >
> > base-commit: 256554692df0685b45e60778b08802b720880c50
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
2026-05-14 17:45 ` Pablo Sabater
@ 2026-05-15 9:33 ` Phillip Wood
0 siblings, 0 replies; 18+ messages in thread
From: Phillip Wood @ 2026-05-15 9:33 UTC (permalink / raw)
To: Pablo Sabater, phillip.wood
Cc: git, gitster, christian.couder, karthik.188, jltobler,
ayu.chandekar, siddharthasthana31, chandrapratap3519
On 14/05/2026 18:45, Pablo Sabater wrote:
> El jue, 14 may 2026 a las 17:15, Phillip Wood
> (<phillip.wood123@gmail.com>) escribió:
>> On 02/04/2026 22:17, Pablo Sabater wrote:
>>> 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, which makes the graph confusing.
>>>
>>> This issue was reported by Junio at:
>>> https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
>>>
>>> e.g.:
>>>
>>> * root-B
>>> * child-A2
>>> * child-A1
>>> * root-A
>>>
>>> [...]
>> >
>>> * root-B
>>> * child-A2
>>> /
>>> * child-A1
>>> * root-A
>>
>> I'm rather late to the party here, but personally I find the indentation
>> a bit confusing, it would be clearer to me if we had a blank line after
>> a root commit
>
> Hi,
>
>>
>> * root-B
>>
>> * child-A2
>> * child-A1
>> * root-A
>>
>> It takes the same amount of vertical space but keeps the children of
>> root-A together.
>
> I have mixed feelings about which approach to choose.
> The idea of a blank line was thought at
> https://lore.kernel.org/git/xmqq8s8vvw9m.fsf@gitster.c.googlers.com/
> but Junio argued against it for having an extra row because the
> indentation he proposed didn't collapse, however I find indentation +
> no collapse the most confusing one.
> I'd say that I'm fine with both approaches, blank line or indentation
> + collapse.
I'm afraid I don't understand this - what does it mean for the
indentation to collapse, or not collapse. Looking at the examples Junio
gave they look quite nice to me, though I'd find it clearer if
| | * 12345678 2021-01-14 merge xxxxx@xxxx into the history
| | |\
| | | \
| | * \ 23456789 2021-01-12 merge citest into the main history
| | |\ * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | | * 3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
Added defau
| | | * ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
f7daf088)
was rendered as
| | * 12345678 2021-01-14 merge xxxxx@xxxx into the history
| | |\
| | | * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
| | * 23456789 2021-01-12 merge citest into the main history
| | |\
| | | * 3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
Added defau
| | | * ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
f7daf088)
>>> without the patch:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> * D root
>>>
>>> with the patch, the indentation cascades:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> _ /
>>> /
>>> /
>>> * D root
>
> * A root
>
> * B root
>
> * C root
>
> * D1 child
>
> * D root
>
> Here I think a blank line looks worse, too much space for just 5
> commits and becomes one extra line which if this were like up to 7 or
> more parentless commits one after the other would be more noticeable.
But there shouldn't be a blank line between D and D1 so the two
alternatives take up the same amount of vertical space, the main
difference being whether D1 appears next to D
* A root * A root
* B root
* B root * C root
* D1 child
* C root _/
/
* D1 child /
* D root * D root
Of course if the indentation was smarter it would take up less room and
look better than having blank lines
* A root
* B root
* C root
* D1 child
* D root
> But there are cases that blank line might be better:
>
> * 10_A2
> * 10_A1
> * 10_A
> * 10_M
> /|\
> | | * 10_D
> | * 10_C
> * 10_B
>
> Feels like a shower of commits instead of an indented merge.
Yes, that is a bit confusing. I think the thing I find confusing with
this approach is that we're treating the commit rendered below the root
commit specially, rather than treating the root commit itself specially.
To me it is the root commit that's the odd one out because it does not
have any parents, but we treat the commit that's rendered below as the
odd one by indenting it relative to its parents.
> Pro to the blank line, the parentless check is the same and it's just
> printing a '\n' at the right spot, while indent i'm mimicking like if
> there was a commit there.
> Anyways, I think in the majority of the cases the indentation +
> collapsing looks better.
> Sorry for the brief reply, I'm busy today.
No need to apologize, it seemed quite comprehensive to me
Thanks
Phillip
> Regards,
>
> --
> Pablo
>
>>
>> Thanks
>>
>> Phillip
>>
>>> This is done by adding a is_placeholder flag to the columns, the root commit
>>> is actually there but marked as a placeholder
>>>
>>> e.g.:
>>>
>>> * root-B
>>> (B) * child-A2
>>> /
>>> * child-A1
>>> * root-A
>>>
>>> (B) would be root-B column with the placeholder flag active.
>>>
>>> Then teaching the rendering function to print a padding ' ' when meeting a
>>> placeholder column outputs the second example.
>>>
>>> There could also be the case where there are multiple roots
>>>
>>> without the patch:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> * D root
>>>
>>> with the patch, the indentation cascades:
>>>
>>> * A root
>>> * B root
>>> * C root
>>> * D1 child
>>> _ /
>>> /
>>> /
>>> * D root
>>>
>>> the _ / might look weird but that's how the collapsing rendering does it
>>> for big gaps, this case being from the 4th column to the 0th column.
>>> Another patch could change the collapsing rendering for placeholders ?
>>> I haven't done it to keep it minimal, but a follow up could make it
>>> to be straight '/'. This would make it bigger but easier for the eye to follow.
>>> IMO is not worth it, but opinions are welcome.
>>>
>>> The patch also adds tests for different cases like a root preceding multiple
>>> parents merges and the examples above.
>>>
>>> There could be some edge cases still so any testing is very welcome.
>>>
>>> Pablo Sabater (1):
>>> graph: add indentation for commits preceded by a root
>>>
>>> graph.c | 68 ++++++++++++++++--
>>> t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
>>> 2 files changed, 198 insertions(+), 6 deletions(-)
>>>
>>>
>>> base-commit: 256554692df0685b45e60778b08802b720880c50
>>
^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2026-05-15 9:33 UTC | newest]
Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-02 21:17 [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Pablo Sabater
2026-04-02 21:17 ` [GSoC RFC PATCH 1/1] " Pablo Sabater
2026-04-03 17:55 ` Junio C Hamano
2026-04-03 18:07 ` Pablo
2026-04-03 5:04 ` [GSoC RFC PATCH 0/1] " Junio C Hamano
2026-04-03 8:25 ` Pablo
2026-04-04 9:24 ` [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit Pablo Sabater
2026-04-04 9:24 ` [GSoC RFC PATCH v2 1/1] " Pablo Sabater
2026-04-10 16:25 ` [GSoC RFC PATCH v2 0/1] " Pablo
2026-04-10 16:54 ` Junio C Hamano
2026-04-27 10:28 ` [GSoC PATCH v3 " Pablo Sabater
2026-04-27 10:28 ` [GSoC PATCH v3 1/1] " Pablo Sabater
2026-05-13 23:02 ` Jeff King
2026-05-14 10:19 ` Pablo Sabater
2026-04-27 10:35 ` [GSoC PATCH v3 0/1] " Pablo
2026-05-14 15:15 ` [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root Phillip Wood
2026-05-14 17:45 ` Pablo Sabater
2026-05-15 9:33 ` Phillip Wood
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox