[GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[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

[GSoC RFC PATCH 1/1] graph: add indentation for commits preceded by a root

[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

Re: [GSoC RFC PATCH 1/1] graph: add indentation for commits preceded by a root

[Junio C Hamano]

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.

Re: [GSoC RFC PATCH 1/1] graph: add indentation for commits preceded by a root

[Pablo]

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.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Junio C Hamano]

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.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Pablo]

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

[GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit

[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

[GSoC RFC PATCH v2 1/1] graph: add indentation for commits preceded by a parentless commit

[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

Re: [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit

[Pablo]

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

Re: [GSoC RFC PATCH v2 0/1] graph: add indentation for commits preceded by a parentless commit

[Junio C Hamano]

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.

[GSoC PATCH v3 0/1] graph: add indentation for commits preceded by a parentless commit

[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

[GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit

[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

Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit

[Jeff King]

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

Re: [GSoC PATCH v3 1/1] graph: add indentation for commits preceded by a parentless commit

[Pablo Sabater]

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

Re: [GSoC PATCH v3 0/1] graph: add indentation for commits preceded by a parentless commit

[Pablo]

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

[PATCH v4 0/2] graph: indent visual roots in graph

[Pablo Sabater]

When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.

A fix for this issue was already attempted [1] a while ago.

This series adds indentation to the visual root commits, so they cannot be
vertically adjacent anymore making it easier to identify them.

before indentation:

	* A
	* B1
	* B2
	* C1
	* C2

after indentation:

	  * A
	* B1
	 \
	  * B2
	* C1
	* C2

Indents the visual root commits that have still commits to show after them, and
if they have children it connects them with an edge at a new row.

If there are multiple visual roots adjacent in history, the indentation starts
with the second one, avoiding redundant indentation of the first one and cascades
after the second.

	* A
	  * B
	    * C
	* D1
	* D2

This series first commit is a cleanup that brings a common function from t4215
and t6016 to a graph functions file which they both use, so the new test file
for indentation, t4218, can use it as well.

The lookahead used to set the cascading and avoid extra indentation is not
completely reliable, as the walker goes through the commits it simplifies the
history of the current commit and its parents, but it doesn't simplify it
for the next unrelated or the grandparents. When the walker simplifies the
history, it removes filtered commits from the history and sets its flags.
When the next commit is an unrelated commit and its parents will be filtered
out, for the lookahead the commit is still a child of, it cannot know that the
next commit once simplified (advancing the walker) it will become a visual root.
This makes the lookahead fail, failing to set the cascading and starting it
with the first visual root, carrying an extra indent for the cascade.

given:

	* A unrelated (visual root)
	* B child of C
	* C visual root WILL BE FILTERED OUT
	* D unrelated (visual root)

the actual output is:

	  * A
	    * B
	* D

A test has been added to t4218 and a NEEDSWORK to the lookahead function to
document this edge case but I'm not that familiar with revision.c. Maybe there's
a better way to make the lookahead more reliable.

[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/

V3 DIFF:

 - Completly changes the approach to indent the visual roots instead of the
   commits after the visual roots.

Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
Pablo Sabater (2):
      lib-log-graph: move check_graph function
      graph: indent visual root in graph

 graph.c                                    | 262 +++++++++++++++++
 t/lib-log-graph.sh                         |   5 +
 t/meson.build                              |   1 +
 t/t4215-log-skewed-merges.sh               |  33 +--
 t/t4218-log-graph-indentation.sh           | 455 +++++++++++++++++++++++++++++
 t/t6016-rev-list-graph-simplify-history.sh |  25 +-
 6 files changed, 747 insertions(+), 34 deletions(-)
---
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
change-id: 20260612-ps-pre-commit-indent-39ca72816382

Best regards,
--  
Pablo Sabater <pabloosabaterr@gmail.com>

[PATCH v4 1/2] lib-log-graph: move check_graph function

[Pablo Sabater]

check_graph is a function shared in the test files t4215 and t6016 used
to format the output graph, but instead of being in a file called by
both test, the function code is repeated in each file.

Move check_graph to lib-log-graph.sh file which both tests already
import graph functions from, renaming it to lib_test_check_graph.

This function is needed for the following commit which includes graph
tests in a new file and requires check_graph.

Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 t/lib-log-graph.sh                         |  5 +++++
 t/t4215-log-skewed-merges.sh               | 33 +++++++++++++-----------------
 t/t6016-rev-list-graph-simplify-history.sh | 25 +++++++++-------------
 3 files changed, 29 insertions(+), 34 deletions(-)

diff --git a/t/lib-log-graph.sh b/t/lib-log-graph.sh
index bf952ef920..1eae8f60c2 100644
--- a/t/lib-log-graph.sh
+++ b/t/lib-log-graph.sh
@@ -26,3 +26,8 @@ lib_test_cmp_colored_graph () {
 	test_decode_color <output.colors.raw | sed "s/ *\$//" >output.colors &&
 	test_cmp expect.colors output.colors
 }
+
+lib_test_check_graph () {
+	cat >expect &&
+	lib_test_cmp_graph --format=%s "$@"
+}
diff --git a/t/t4215-log-skewed-merges.sh b/t/t4215-log-skewed-merges.sh
index 1612f05f1b..eebab71039 100755
--- a/t/t4215-log-skewed-merges.sh
+++ b/t/t4215-log-skewed-merges.sh
@@ -5,11 +5,6 @@ test_description='git log --graph of skewed merges'
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-log-graph.sh
 
-check_graph () {
-	cat >expect &&
-	lib_test_cmp_graph --format=%s "$@"
-}
-
 test_expect_success 'log --graph with merge fusing with its left and right neighbors' '
 	git checkout --orphan _p &&
 	test_commit A &&
@@ -21,7 +16,7 @@ test_expect_success 'log --graph with merge fusing with its left and right neigh
 	git checkout _p && git merge --no-ff _r -m G &&
 	git checkout @^^ && git merge --no-ff _p -m H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   H
 	|\
 	| *   G
@@ -49,7 +44,7 @@ test_expect_success 'log --graph with left-skewed merge' '
 	git checkout 0_p && git merge --no-ff 0_s -m 0_G &&
 	git checkout @^ && git merge --no-ff 0_q 0_r 0_t 0_p -m 0_H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*-----.   0_H
 	|\ \ \ \
 	| | | | * 0_G
@@ -83,7 +78,7 @@ test_expect_success 'log --graph with nested left-skewed merge' '
 	git checkout 1_p && git merge --no-ff 1_r -m 1_G &&
 	git checkout @^^ && git merge --no-ff 1_p -m 1_H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   1_H
 	|\
 	| *   1_G
@@ -115,7 +110,7 @@ test_expect_success 'log --graph with nested left-skewed merge following normal
 	git checkout -b 2_s @^^ && git merge --no-ff 2_q -m 2_J &&
 	git checkout 2_p && git merge --no-ff 2_s -m 2_K &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   2_K
 	|\
 	| *   2_J
@@ -151,7 +146,7 @@ test_expect_success 'log --graph with nested right-skewed merge following left-s
 	git checkout 3_p && git merge --no-ff 3_r -m 3_H &&
 	git checkout @^^ && git merge --no-ff 3_p -m 3_J &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   3_J
 	|\
 	| *   3_H
@@ -182,7 +177,7 @@ test_expect_success 'log --graph with right-skewed merge following a left-skewed
 	git merge --no-ff 4_p -m 4_G &&
 	git checkout @^^ && git merge --no-ff 4_s -m 4_H &&
 
-	check_graph --date-order <<-\EOF
+	lib_test_check_graph --date-order <<-\EOF
 	*   4_H
 	|\
 	| *   4_G
@@ -218,7 +213,7 @@ test_expect_success 'log --graph with octopus merge with column joining its penu
 	git checkout 5_r &&
 	git merge --no-ff 5_s -m 5_H &&
 
-	check_graph <<-\EOF
+	lib_test_check_graph <<-\EOF
 	*   5_H
 	|\
 	| *-.   5_G
@@ -257,7 +252,7 @@ test_expect_success 'log --graph with multiple tips' '
 	git checkout 6_1 &&
 	git merge --no-ff 6_2 -m 6_I &&
 
-	check_graph 6_1 6_3 6_5 <<-\EOF
+	lib_test_check_graph 6_1 6_3 6_5 <<-\EOF
 	*   6_I
 	|\
 	| | *   6_H
@@ -334,7 +329,7 @@ test_expect_success 'log --graph with multiple tips' '
 	git checkout -b M_7 7_1 &&
 	git merge --no-ff 7_2 7_3 -m 7_M4 &&
 
-	check_graph M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
@@ -371,7 +366,7 @@ test_expect_success 'log --graph with multiple tips' '
 '
 
 test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
-	check_graph --graph-lane-limit=2 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=2 M_7 <<-\EOF
 	*-.   7_M4
 	|\ \
 	| | * 7_G
@@ -388,7 +383,7 @@ test_expect_success 'log --graph --graph-lane-limit=2 limited to two lanes' '
 '
 
 test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge' '
-	check_graph --graph-lane-limit=1 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=1 M_7 <<-\EOF
 	*-~  7_M4
 	|\~
 	| ~ 7_G
@@ -405,7 +400,7 @@ test_expect_success 'log --graph --graph-lane-limit=1 truncate mid octopus merge
 '
 
 test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
-	check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=3 M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
@@ -441,7 +436,7 @@ test_expect_success 'log --graph --graph-lane-limit=3 limited to three lanes' '
 '
 
 test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows first of 3 parent merge' '
-	check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=6 M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
@@ -478,7 +473,7 @@ test_expect_success 'log --graph --graph-lane-limit=6 check if it only shows fir
 '
 
 test_expect_success 'log --graph --graph-lane-limit=7 check if it shows all 3 parent merge' '
-	check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
+	lib_test_check_graph --graph-lane-limit=7 M_1 M_3 M_5 M_7 <<-\EOF
 	*   7_M1
 	|\
 	| | *   7_M2
diff --git a/t/t6016-rev-list-graph-simplify-history.sh b/t/t6016-rev-list-graph-simplify-history.sh
index 54b0a6f5f8..e0d9c3c1ac 100755
--- a/t/t6016-rev-list-graph-simplify-history.sh
+++ b/t/t6016-rev-list-graph-simplify-history.sh
@@ -13,11 +13,6 @@ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
 . ./test-lib.sh
 . "$TEST_DIRECTORY"/lib-log-graph.sh
 
-check_graph () {
-	cat >expect &&
-	lib_test_cmp_graph --format=%s "$@"
-}
-
 test_expect_success 'set up rev-list --graph test' '
 	# 3 commits on branch A
 	test_commit A1 foo.txt &&
@@ -54,7 +49,7 @@ test_expect_success 'set up rev-list --graph test' '
 '
 
 test_expect_success '--graph --all' '
-	check_graph --all <<-\EOF
+	lib_test_check_graph --all <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -82,7 +77,7 @@ test_expect_success '--graph --all' '
 # that undecorated merges are interesting, even with --simplify-by-decoration
 test_expect_success '--graph --simplify-by-decoration' '
 	git tag -d A4 &&
-	check_graph --all --simplify-by-decoration <<-\EOF
+	lib_test_check_graph --all --simplify-by-decoration <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -114,7 +109,7 @@ test_expect_success 'setup: get rid of decorations on B' '
 
 # Graph with branch B simplified away
 test_expect_success '--graph --simplify-by-decoration prune branch B' '
-	check_graph --simplify-by-decoration --all <<-\EOF
+	lib_test_check_graph --simplify-by-decoration --all <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -133,7 +128,7 @@ test_expect_success '--graph --simplify-by-decoration prune branch B' '
 '
 
 test_expect_success '--graph --full-history -- bar.txt' '
-	check_graph --full-history --all -- bar.txt <<-\EOF
+	lib_test_check_graph --full-history --all -- bar.txt <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -148,7 +143,7 @@ test_expect_success '--graph --full-history -- bar.txt' '
 '
 
 test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
-	check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
+	lib_test_check_graph --full-history --simplify-merges --all -- bar.txt <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -161,7 +156,7 @@ test_expect_success '--graph --full-history --simplify-merges -- bar.txt' '
 '
 
 test_expect_success '--graph -- bar.txt' '
-	check_graph --all -- bar.txt <<-\EOF
+	lib_test_check_graph --all -- bar.txt <<-\EOF
 	* A7
 	* A5
 	* A3
@@ -172,7 +167,7 @@ test_expect_success '--graph -- bar.txt' '
 '
 
 test_expect_success '--graph --sparse -- bar.txt' '
-	check_graph --sparse --all -- bar.txt <<-\EOF
+	lib_test_check_graph --sparse --all -- bar.txt <<-\EOF
 	* A7
 	* A6
 	* A5
@@ -189,7 +184,7 @@ test_expect_success '--graph --sparse -- bar.txt' '
 '
 
 test_expect_success '--graph ^C4' '
-	check_graph --all ^C4 <<-\EOF
+	lib_test_check_graph --all ^C4 <<-\EOF
 	* A7
 	* A6
 	* A5
@@ -202,7 +197,7 @@ test_expect_success '--graph ^C4' '
 '
 
 test_expect_success '--graph ^C3' '
-	check_graph --all ^C3 <<-\EOF
+	lib_test_check_graph --all ^C3 <<-\EOF
 	* A7
 	*   A6
 	|\
@@ -220,7 +215,7 @@ test_expect_success '--graph ^C3' '
 # that important, but this test depends on it.  If the ordering ever changes
 # in the code, we'll need to update this test.
 test_expect_success '--graph --boundary ^C3' '
-	check_graph --boundary --all ^C3 <<-\EOF
+	lib_test_check_graph --boundary --all ^C3 <<-\EOF
 	* A7
 	*   A6
 	|\

-- 
2.54.0

[PATCH v4 2/2] graph: indent visual root in graph

[Pablo Sabater]

When rendering a graph, if the history contains multiple "visual roots",
actual roots or commits that look like roots (i.e. have their parents
filtered out) can end up being vertically adjacent to unrelated commits,
falsely appearing to be related.

A fix for this issue was already attempted [1] a while ago.

This happens because the commits fill the space from left to right and
when a visual root ends, its column becomes free for the following
commit even if they are not related. Once this happens the unrelated
commit is rendered below the visual root. Because there is no special
character or way to identify when a visual root is rendered making the
graph confusing.

By indenting the visual roots when there are still commits to show the
vertical adjacency can be avoided.

Add is_visual_root flag to git_graph making it visible in all graph states,
give graph_update() a new function, graph_is_visual_root() to know if the
current commit is a visual root and set is_visual_root.
The different handled cases are:

- If a visual root has children: similar to GRAPH_PRE_COMMIT state when
  octopus merges need space, an edge row needs to be printed to connect
  the child with the indented visual root. A new state GRAPH_PRE_ROOT is
  needed to connect the child with the visual root:

    * child of the visual root
     \ GRAPH_PRE_ROOT
      * visual root indented

- If a visual root is child-less we can skip GRAPH_PRE_ROOT state and
  render the indented commit directly.

      * visual root indented
    * unrelated commit

- If two or more visual roots are adjacent: by having a lookahead to the
  next commit that will be rendered, if the next commit is also a visual
  root and we are on a visual root, meaning two visual root adjacent in
  the history, the top one can omit the indent, making the one below to
  indent only once, if there are more adjacent visual commits, the
  indentation will increase for each adjacent one, cascading.

    * visual root
      * visual root
        * visual root
    * last commit

  Even if the last commit is a root, because there is nothing that will be
  rendered below we can omit the indentation on purpose.

The lookahead is not completely reliable, on graphs with filtered parents,
the walker when processing the current commit it will simplify its
parents by removing the ones that won't be shown, (They have the
TREESAME flag when filtering by path for example), but it doesn't act
for the grandparents or the next commit if it is unrelated until we move
to the next.

For example given

  A visual root
  B child
  C parent of B, visual root FILTERED
  D last commit

We would expect

  A
    B
  D

When processing A, for the walker and the information at the renderer, B
is still a child of C, as B parent, hasn't been removed yet. This makes
cascade to not trigger as the lookahead fails to detect if the next
commit will be a visual root.
Once at B, its parent has been removed and has become a visual root, and
it just adds its indent to the one left by A. We end up with an extra
indent:

    A
      B
  D

The output isn't broken as unrelated commits are successfully separated
by indentation, but an indent level should have been avoided.

Create a new test file for graph indentations test called
't4218-log-graph-indentation.sh'.

The filtered parents edge case is documented as a NEEDSWORK on the
lookahead function and it has its own 'test_expect_failure' at 't4218'.

[1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/

Mentored-by: Karthik Nayak <karthik.188@gmail.com>
Mentored-by: Chandra Pratap <chandrapratap3519@gmail.com>
Signed-off-by: Pablo Sabater <pabloosabaterr@gmail.com>
---
 graph.c                          | 262 ++++++++++++++++++++++
 t/meson.build                    |   1 +
 t/t4218-log-graph-indentation.sh | 455 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 718 insertions(+)

diff --git a/graph.c b/graph.c
index 842282685f..e0d1e2a510 100644
--- a/graph.c
+++ b/graph.c
@@ -60,12 +60,23 @@ struct column {
 	 * index into column_colors.
 	 */
 	unsigned short color;
+	/*
+	 * Marks if a commit is a non-first parent of a merge. These columns are
+	 * already visually connected to the merge commit and do not need
+	 * indentation.
+	 *
+	 * The first parent is the one that inherits the column and it can need
+	 * indentation if turns out to be a visual root and there's still
+	 * commits to render.
+	 */
+	unsigned is_merge_parent:1;
 };
 
 enum graph_state {
 	GRAPH_PADDING,
 	GRAPH_SKIP,
 	GRAPH_PRE_COMMIT,
+	GRAPH_PRE_ROOT,
 	GRAPH_COMMIT,
 	GRAPH_POST_MERGE,
 	GRAPH_COLLAPSING
@@ -315,6 +326,48 @@ struct git_graph {
 	 * diff_output_prefix_callback().
 	 */
 	struct strbuf prefix_buf;
+
+	/*
+	 * If a commit is a visual root, we need to indent it to prevent
+	 * unrelated commits from being vertically adjacent to it.
+	 */
+	unsigned is_visual_root:1;
+
+	/*
+	 * Indentation increases for each visual root adjacent to another visual
+	 * root, making visual root commits indentation cascade.
+	 */
+	unsigned int visual_root_depth;
+
+	/*
+	 * When a visual root is adjacent to other visual roots, the first one
+	 * can avoid indentation and the rest cascades, increasing the indentation
+	 * for each one.
+	 */
+	unsigned visual_root_cascade:1;
+
+	/*
+	 * Set when the current commit was already present in graph->columns
+	 * before being processed.
+	 */
+	unsigned commit_in_columns:1;
+};
+
+struct graph_lookahead_flags {
+	/*
+	 * Set when there will be a commit after the current one that will be
+	 * rendered.
+	 */
+	unsigned int is_next_visible:1;
+	/*
+	 * Set when the next visible commit is candidate to be a visual root.
+	 */
+	unsigned int is_next_visual_root:1;
+	/*
+	 * Set when the next visible commit will be rendered under the current
+	 * commit.
+	 */
+	unsigned int next_has_column:1;
 };
 
 static inline int graph_needs_truncation(struct git_graph *graph, int lane)
@@ -388,6 +441,8 @@ struct git_graph *graph_init(struct rev_info *opt)
 	graph->num_columns = 0;
 	graph->num_new_columns = 0;
 	graph->mapping_size = 0;
+	graph->visual_root_depth = 0;
+	graph->visual_root_cascade = 0;
 	/*
 	 * Start the column color at the maximum value, since we'll
 	 * always increment it for the first commit we output.
@@ -561,6 +616,11 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 					  struct commit *commit,
 					  int idx)
 {
+	/*
+	 * Get the initial merge_layout before it's modified to know if this
+	 * is a merge.
+	 */
+	int initial_merge_layout = graph->merge_layout;
 	int i = graph_find_new_column_by_commit(graph, commit);
 	int mapping_idx;
 
@@ -572,6 +632,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_merge_parent = 0;
 	}
 
 	if (graph->num_parents > 1 && idx > -1 && graph->merge_layout == -1) {
@@ -610,6 +671,12 @@ static void graph_insert_into_new_columns(struct git_graph *graph,
 	}
 
 	graph->mapping[mapping_idx] = i;
+
+	/*
+	 * Mark non-first parents of a merge.
+	 */
+	if (graph->num_parents > 1 && initial_merge_layout >= 0 && idx > -1)
+		graph->new_columns[i].is_merge_parent = 1;
 }
 
 static void graph_update_columns(struct git_graph *graph)
@@ -701,10 +768,20 @@ static void graph_update_columns(struct git_graph *graph)
 			if (graph->num_parents == 0)
 				graph->width += 2;
 		} else {
+			int j;
 			graph_insert_into_new_columns(graph, col_commit, -1);
+			/*
+			 * This column is not the current commit, but we need to
+			 * propagate the flag until the commit is processed.
+			 */
+			j = graph_find_new_column_by_commit(graph, col_commit);
+			if (j >= 0 && graph->columns[i].is_merge_parent)
+				graph->new_columns[j].is_merge_parent = 1;
 		}
 	}
 
+	graph->commit_in_columns = is_commit_in_columns;
+
 	/*
 	 * If graph_max_lanes is set, cap the width
 	 */
@@ -763,9 +840,135 @@ static int graph_needs_pre_commit_line(struct git_graph *graph)
 	       graph->expansion_row < graph_num_expansion_rows(graph);
 }
 
+/*
+ * A commit can be a visual root when:
+ * - It has no parents.
+ *
+ * - It has parents but they are all filtered out and
+ *   commit->parents arrives NULL.
+ *
+ * - It is not a boundary commit. Boundary commits also have no visible
+ *   parents, but they are not selected as visual roots because they cannot
+ *.  cause the ambiguity of being vertically adjacent because:
+ *
+ *   1. A boundary only appears 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 commits cannot 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.
+ */
+static int graph_is_visual_root_candidate(struct commit *c)
+{
+	return c->parents == NULL && !(c->object.flags & BOUNDARY);
+}
+
+static int graph_is_visual_root(struct git_graph *graph,
+				struct graph_lookahead_flags *flags)
+{
+	/*
+	 * This must be only called for the current commit as graph contains
+	 * the state for the current commit only.
+	 *
+	 * To check if a commit is a visual root, call graph_is_visual_root_candidate()
+	 * but we won't know if it is really a visual root until we get to the
+	 * next commit state.
+	 *
+	 * The current commit is an actual visual root if it is a candidate and
+	 * the commit is not a non-first parent of a merge.
+	 *
+	 *   *
+	 *   |\
+	 *   | *    <- it is a visual root candidate but it shouldn't be indented
+	 *   *         because it is already connected by an edge.
+	 *   ^         if commit_in_columns && is_merge_parent means the commit
+	 *   |         was put by a merge and is connected.
+	 *   |
+	 *   `-------- if !is_next_visible means we're on the last commit, avoid
+	 *             indentation unless the one before is a visual root, then
+	 *             we need to differentiate from the one above.
+	 *
+	 * If next_has_columns means that the next commit has
+	 * already a column, so it will not be rendered below, the
+	 * current commit has to act as the last commit and omit
+	 * indentation.
+	 */
+	return graph_is_visual_root_candidate(graph->commit) &&
+	       !(graph->commit_in_columns &&
+		 graph->columns[graph->commit_index].is_merge_parent) &&
+	       flags->is_next_visible &&
+	       (!flags->next_has_column || graph->visual_root_depth > 0);
+}
+
+/*
+ * Iterates the commits queue searching for the next visible commit, once found
+ * sets visibleness and visual-root flags.
+ * Knowing if the next commit is also a visual root avoids redundant indentations
+ *
+ * NEEDSWORK: The queue is actively being modified by the walker, for each commit
+ * its parents and itself get simplified and their flags set, but for the next
+ * unrelated commit or the grandparents they are not simplified yet, which means
+ * that a commit whose parents are all filtered will not be marked as a visual
+ * root candidate at the lookahead.
+ * This causes the lookahead to fail, failing to set the cascade flag to avoid
+ * redundant indentations.
+ * See 'test_expect_failure' at t4218-log-graph-indentation.sh.
+ */
+static void graph_peek_next_visible(struct git_graph *graph,
+				    struct graph_lookahead_flags *flags)
+{
+	struct commit_list *cl;
+
+	flags->is_next_visible = 0;
+	flags->is_next_visual_root = 0;
+	flags->next_has_column = 0;
+
+	for (cl = graph->revs->commits; cl; cl = cl->next) {
+		if (get_commit_action(graph->revs, cl->item) != commit_show)
+			continue;
+		flags->is_next_visible = 1;
+		flags->next_has_column = graph_find_new_column_by_commit(graph, cl->item) >= 0;
+		/*
+		 * We do not need graph->commit_in_columns or is_merge_parent,
+		 * because we only need to know whether the next one might be a
+		 * visual root, affecting the current commit where the cascade
+		 * would have to be set and the first visual root not indented.
+		 *
+		 * It will set next_is_visual_root to true for merge parents that
+		 * graph_is_visual_root() would return false, but if the next is
+		 * a merge parent, the current commit is the child and cannot
+		 * be a visual root and therefore having no effect.
+		 */
+		if (!graph_is_visual_root_candidate(cl->item))
+			return;
+
+		/*
+		 * The next visible commit is a visual root candidate, but
+		 * only set cascade if it's not the last commit to be rendered.
+		 */
+		for (cl = cl->next; cl; cl = cl->next) {
+			if (get_commit_action(graph->revs, cl->item) != commit_show)
+				continue;
+			flags->is_next_visual_root = 1;
+			return;
+		}
+		return;
+	}
+}
+
+static int graph_needs_pre_root_line(struct git_graph *graph)
+{
+	return graph->commit_in_columns && graph->is_visual_root &&
+	       graph->num_columns > 0 && !graph->visual_root_cascade;
+}
+
 void graph_update(struct git_graph *graph, struct commit *commit)
 {
 	struct commit_list *parent;
+	struct graph_lookahead_flags flags;
 
 	/*
 	 * Set the new commit
@@ -796,6 +999,23 @@ void graph_update(struct git_graph *graph, struct commit *commit)
 	 */
 	graph_update_columns(graph);
 
+	graph_peek_next_visible(graph, &flags);
+
+	graph->is_visual_root = graph_is_visual_root(graph, &flags);
+
+	if (graph->is_visual_root) {
+		/*
+		 * If next is a visual root we can omit the indent for the first
+		 * visual root and start cascading.
+		 */
+		if (!graph->visual_root_depth && flags.is_next_visual_root)
+			graph->visual_root_cascade = 1;
+		graph->visual_root_depth++;
+	} else {
+		graph->visual_root_depth = 0;
+		graph->visual_root_cascade = 0;
+	}
+
 	graph->expansion_row = 0;
 
 	/*
@@ -813,11 +1033,16 @@ void graph_update(struct git_graph *graph, struct commit *commit)
 	 * room for it.  We need to do this only if there is a branch row
 	 * (or more) to the right of this commit.
 	 *
+	 * If it is a visual root, we need to print an extra row to
+	 * connect the indentation.
+	 *
 	 * If there are less than 3 parents, we can immediately print the
 	 * commit line.
 	 */
 	if (graph->state != GRAPH_PADDING)
 		graph->state = GRAPH_SKIP;
+	else if (graph_needs_pre_root_line(graph))
+		graph->state = GRAPH_PRE_ROOT;
 	else if (graph_needs_pre_commit_line(graph))
 		graph->state = GRAPH_PRE_COMMIT;
 	else
@@ -1065,6 +1290,17 @@ static void graph_output_commit_line(struct git_graph *graph, struct graph_line
 
 		if (col_commit == graph->commit) {
 			seen_this = 1;
+			if (graph->is_visual_root) {
+				int depth = graph->visual_root_depth;
+				/*
+				 * Each visual column is 2 characters wide.
+				 * Omit the indentation for the first visual
+				 * root in cascade mode.
+				 */
+				int padding = (depth - graph->visual_root_cascade) * 2;
+				graph_line_addchars(line, ' ', padding);
+				graph->width += padding;
+			}
 			graph_output_commit_char(graph, line);
 
 			if (graph_needs_truncation(graph, i)) {
@@ -1436,6 +1672,29 @@ static void graph_output_collapsing_line(struct git_graph *graph, struct graph_l
 		graph_update_state(graph, GRAPH_PADDING);
 }
 
+static void graph_output_pre_root_line(struct git_graph *graph, struct graph_line *line)
+{
+	/*
+	 * This function adds a row before a visual root, to connect the
+	 * branch to the indented commit. It should only be called on a
+	 * visual root.
+	 */
+	assert(graph->is_visual_root);
+
+	for (size_t i = 0; i < graph->num_columns; i++) {
+		struct column *col = &graph->columns[i];
+		if (col->commit == graph->commit) {
+			graph_line_addch(line, ' ');
+			graph_line_write_column(line, col, '\\');
+		} else {
+			graph_line_write_column(line, col, '|');
+		}
+		graph_line_addch(line, ' ');
+	}
+
+	graph_update_state(graph, GRAPH_COMMIT);
+}
+
 int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 {
 	int shown_commit_line = 0;
@@ -1461,6 +1720,9 @@ int graph_next_line(struct git_graph *graph, struct strbuf *sb)
 	case GRAPH_PRE_COMMIT:
 		graph_output_pre_commit_line(graph, &line);
 		break;
+	case GRAPH_PRE_ROOT:
+		graph_output_pre_root_line(graph, &line);
+		break;
 	case GRAPH_COMMIT:
 		graph_output_commit_line(graph, &line);
 		shown_commit_line = 1;
diff --git a/t/meson.build b/t/meson.build
index c5832fee05..17037a8465 100644
--- a/t/meson.build
+++ b/t/meson.build
@@ -576,6 +576,7 @@ integration_tests = [
   't4215-log-skewed-merges.sh',
   't4216-log-bloom.sh',
   't4217-log-limit.sh',
+  't4218-log-graph-indentation.sh',
   't4252-am-options.sh',
   't4253-am-keep-cr-dos.sh',
   't4254-am-corrupt.sh',
diff --git a/t/t4218-log-graph-indentation.sh b/t/t4218-log-graph-indentation.sh
new file mode 100755
index 0000000000..f1c9584ba5
--- /dev/null
+++ b/t/t4218-log-graph-indentation.sh
@@ -0,0 +1,455 @@
+#!/bin/sh
+
+test_description='git log --graph visual root indentations'
+
+. ./test-lib.sh
+. "$TEST_DIRECTORY"/lib-log-graph.sh
+
+check_graph_with_description () {
+	cat >expect &&
+	lib_test_cmp_graph --format="%s%ndescription%nsecond-line" "$@"
+}
+
+create_orphan () {
+	git checkout --orphan "$1" &&
+	{ git rm -rf . || true; }
+}
+
+test_expect_success 'single root commit is not indented' '
+	create_orphan _1 && test_commit 1_A &&
+	lib_test_check_graph _1 <<-\EOF
+	* 1_A
+	EOF
+'
+
+test_expect_success 'visual root indented before unrelated branch' '
+	create_orphan _2 && test_commit 2_A && test_commit 2_B &&
+	create_orphan _3 && test_commit 3_A &&
+	lib_test_check_graph _2 _3 <<-\EOF
+	  * 3_A
+	* 2_B
+	* 2_A
+	EOF
+'
+
+test_expect_success 'visual root indentation with --left-right' '
+	lib_test_check_graph --left-right _2..._3 <<-\EOF
+	  > 3_A
+	< 2_B
+	< 2_A
+	EOF
+'
+
+# A better case of why indentation is still needed with '--left-right' flag is
+# that unrelated branches can be on the same side, so it's needed to
+# differentiate visual roots on the same side.
+test_expect_success 'visual root indentation with --left-right having unrelated commits on the same side' '
+	lib_test_check_graph --left-right _2..._3 _1 <<-\EOF
+	  > 3_A
+	< 2_B
+	 \
+	  < 2_A
+	> 1_A
+	EOF
+'
+
+test_expect_success 'visual root indents the description also' '
+	check_graph_with_description _2 _3 <<-\EOF
+	  * 3_A
+	    description
+	    second-line
+	* 2_B
+	| description
+	| second-line
+	* 2_A
+	  description
+	  second-line
+	EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child' '
+	create_orphan _4 && test_commit 4_A && test_commit 4_B &&
+	create_orphan _5 && test_commit 5_A && test_commit 5_B &&
+	lib_test_check_graph _4 _5<<-\EOF
+	* 5_B
+	 \
+	  * 5_A
+	* 4_B
+	* 4_A
+	EOF
+'
+
+test_expect_success 'indented visual root parent gets connected to its child with description' '
+	check_graph_with_description _4 _5 <<-\EOF
+	* 5_B
+	| description
+	| second-line
+	 \
+	  * 5_A
+	    description
+	    second-line
+	* 4_B
+	| description
+	| second-line
+	* 4_A
+	  description
+	  second-line
+	EOF
+'
+
+test_expect_success 'visual roots cascade and last root does not' '
+	create_orphan _7 && test_commit 7_A && test_commit 7_B &&
+	create_orphan _8 && test_commit 8_A &&
+	create_orphan _9 && test_commit 9_A &&
+	create_orphan _10 && test_commit 10_A &&
+	lib_test_check_graph _7 _8 _9 _10  <<-\EOF
+	* 10_A
+	  * 9_A
+	    * 8_A
+	* 7_B
+	* 7_A
+	EOF
+'
+
+test_expect_success 'last root does not cascade' '
+	lib_test_check_graph _8 _9 _10 <<-\EOF
+	* 10_A
+	  * 9_A
+	* 8_A
+	EOF
+'
+
+test_expect_success 'merge parents are roots between them but they do not indent' '
+	create_orphan _11 && test_commit 11_A &&
+	create_orphan _12 && test_commit 12_A &&
+	create_orphan _13 && test_commit 13_A &&
+	git checkout _11 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _11 -p _12 -p _13 -m 11_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _11 <<-\EOF
+	*-.   11_octopus
+	|\ \
+	| | * 13_A
+	| * 12_A
+	* 11_A
+	EOF
+'
+
+# The last parent of a merge can be indented if nothing related to it needs to
+# be rendered after, if it's another visual root, merge parent must not get
+# indented but rather activate cascading.
+test_expect_success 'merge then unrelated visual root and unrelated branch' '
+	create_orphan _16 && test_commit 16_A && test_commit 16_B &&
+	create_orphan _17 && test_commit 17_A &&
+	create_orphan _18 && test_commit 18_A &&
+	create_orphan _19 && test_commit 19_A &&
+	create_orphan _20 && test_commit 20_A &&
+	git checkout _18 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _18 -p _19 -p _20 -m 18_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _18 _17 _16 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	* 18_A
+	  * 17_A
+	* 16_B
+	* 16_A
+	EOF
+'
+
+# The last commit root does not get indented, if the next thing after the root
+# merge parent is the last commit, indent the merge parent.
+test_expect_success 'merge then unrelated root indents merge parent' '
+	lib_test_check_graph _18 _17 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	 \
+	  * 18_A
+	* 17_A
+	EOF
+'
+
+test_expect_success 'merge then unrelated branch indents merge parent' '
+	lib_test_check_graph _18 _16 <<-\EOF
+	*-.   18_octopus
+	|\ \
+	| | * 20_A
+	| * 19_A
+	 \
+	  * 18_A
+	* 16_B
+	* 16_A
+	EOF
+'
+
+test_expect_success 'two-parent merge of orphans' '
+	create_orphan _21 && test_commit 21_A &&
+	create_orphan _22 && test_commit 22_A &&
+	git checkout _21 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _21 -p _22 -m 21_merge) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph _21 <<-\EOF
+	*   21_merge
+	|\
+	| * 22_A
+	* 21_A
+	EOF
+'
+
+test_expect_success 'commit with filtered parent becomes a visual root' '
+	create_orphan _23 &&
+	echo test >other.txt &&
+	git add other.txt &&
+	git commit -m "23_A" &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "23_B" &&
+	create_orphan _24 &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "24_A" &&
+	lib_test_check_graph _23 _24 -- foo.txt <<-\EOF
+	  * 23_B
+	* 24_A
+	EOF
+'
+
+# The walker simplifies the commit for the current one and its parents, removing
+# the filtered parents, but it doesn't go one step ahead, this causes some edge
+# cases with the lookahead.
+# Given A (orphan), the walker only processes A, and when we lookahead for B
+# (child of C) even tho C will be filtered, it hasn't been simplified yet, so we
+# don't see B as a visual root, therefore cascade indentation isn't applied to A.
+# (cascade indentation starts the indentation at the second visual root, to avoid
+# redundant indentation). So A gets an extra indent, and once B is processed,
+# when rendering it, C has been removed, B is a visual root and as the last commit
+# isn't considered a visual root as it cannot have unrelated commits below it,
+# cascading isn't also applied, giving B another indent.
+#
+# The final result is an extra indent for A and B:
+#
+#	  A
+#	    B
+#	D
+#
+# This will happen for any case where we find ourselves with the next commit
+# being a unrelated child of a parent the will be filtered.
+#
+# instead of the expected:
+test_expect_failure 'filtered parent cascading edge case' '
+	create_orphan _25 &&
+	git rm -rf . &&
+	echo test >other.txt &&
+	git add other.txt &&
+	git commit -m "C-filtered" &&
+
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "B (child of filtered)" &&
+
+	create_orphan _26 &&
+	git rm -rf . &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "A (visual root)" &&
+
+	create_orphan _27 &&
+	git rm -rf . &&
+	echo test >foo.txt &&
+	git add foo.txt &&
+	git commit -m "D (last)" &&
+
+	lib_test_check_graph _25 _26 _27 -- foo.txt <<-\EOF
+	* A (visual root)
+	  * B (child of filtered)
+	* D (last)
+	EOF
+'
+
+test_expect_failure 'multiple filtered parents in sequence' '
+	create_orphan _44 &&
+	git rm -rf . &&
+	echo a >other.txt && git add other.txt && git commit -m "44_F" &&
+	echo b >foo.txt && git add foo.txt && git commit -m "44_C" &&
+
+	create_orphan _45 &&
+	git rm -rf . &&
+	echo c >other.txt && git add other.txt && git commit -m "45_F" &&
+	echo d >foo.txt && git add foo.txt && git commit -m "45_C" &&
+
+	create_orphan _46 &&
+	git rm -rf . &&
+	echo e >foo.txt && git add foo.txt && git commit -m "46_A" &&
+
+	lib_test_check_graph _44 _45 _46 -- foo.txt <<-\EOF
+	* 46_A
+	  * 45_C
+	* 44_C
+	EOF
+'
+
+test_expect_failure 'real orphan root followed by child of filtered parent' '
+	create_orphan _47 &&
+	git rm -rf . &&
+	echo a >foo.txt && git add foo.txt && git commit -m "47_A" &&
+
+	create_orphan _48 &&
+	git rm -rf . &&
+	echo b >other.txt && git add other.txt && git commit -m "48_filtered" &&
+	echo c >foo.txt && git add foo.txt && git commit -m "48_B" &&
+
+	create_orphan _49 &&
+	git rm -rf . &&
+	echo d >foo.txt && git add foo.txt && git commit -m "49_last" &&
+
+	lib_test_check_graph _47 _48 _49 -- foo.txt <<-\EOF
+	* 47_A
+	  * 48_B
+	* 49_last
+	EOF
+'
+
+# This tests prove why there is no need to have indentation for boundary
+# commits.
+#
+# Boundary commits rather than starting a column they 'inherit' the one of
+# its child so there will always be an edge that connects it removing the
+# ambiguity.
+test_expect_success 'unrelated boundaries are not ambiguous' '
+	create_orphan _28 && test_commit 28_A && test_commit 28_B &&
+	test_commit 28_C &&
+	create_orphan _29 && test_commit 29_A && test_commit 29_B &&
+	lib_test_check_graph --boundary 28_A.._28 29_A.._29 <<-\EOF
+	* 29_B
+	| * 28_C
+	| * 28_B
+	| o 28_A
+	o 29_A
+	EOF
+'
+
+# Same structure as t6016
+test_expect_success 'boundary commits big test' '
+	# 3 commits on branch _30
+	create_orphan _30 &&
+	test_commit 30_A &&
+	test_commit 30_B &&
+	test_commit 30_C &&
+
+	# 2 commits on branch _31, started from 30_A
+	git checkout -b _31 30_A &&
+	test_commit 31_A &&
+	test_commit 31_B &&
+
+	# 2 commits on branch _32, started from 30_B
+	git checkout -b _32 30_B &&
+	test_commit 32_A &&
+	test_commit 32_B &&
+
+	# Octopus merge _31 and _32 into -30
+	git checkout _30 &&
+	git merge _31 _32 -m 30_D &&
+	git tag 30_D &&
+	test_commit 30_E &&
+
+	# More commits on _32, then merge _32 into _30
+	git checkout _32 &&
+	test_commit 32_C &&
+	test_commit 32_D &&
+	git checkout _30 &&
+	git merge -s ours _32 -m 30_F &&
+	git tag 30_F &&
+	test_commit 30_G &&
+	lib_test_check_graph --boundary _30 _31 _32 ^32_C <<-\EOF
+	* 30_G
+	*   30_F
+	|\
+	| * 32_D
+	* | 30_E
+	| |
+	|  \
+	*-. \   30_D
+	|\ \ \
+	| * | | 31_B
+	| * | | 31_A
+	* | | | 30_C
+	o | | | 30_B
+	|/ / /
+	o / / 30_A
+	 / /
+	| o 32_C
+	|/
+	o 32_B
+	EOF
+'
+
+# Filter by --first-parent and then forcing the filtered parents to be shown.
+test_expect_success '--first-parent flag with the filtered parents' '
+	(
+	create_orphan _35 && test_commit 35_A && test_commit 35_B &&
+	create_orphan _36 && test_commit 36_A &&
+	create_orphan _37 && test_commit 37_A &&
+	git checkout _35 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _35 -p _36 -p _37 -m 35_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph --first-parent _35 _36 _37 <<-\EOF
+	* 35_octopus
+	| * 37_A
+	|   * 36_A
+	* 35_B
+	* 35_A
+	EOF
+	)
+'
+
+test_expect_success '--first-parent with filtered parents but one has a child' '
+	(
+	create_orphan _38 && test_commit 38_A && test_commit 38_B &&
+	create_orphan _39 && test_commit 39_A &&
+	create_orphan _40 && test_commit 40_A && test_commit 40_B &&
+	git checkout _38 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _38 -p _39 -p _40 -m 38_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph --first-parent _38 _39 _40 <<-\EOF
+	* 38_octopus
+	| * 40_B
+	| * 40_A
+	|   * 39_A
+	* 38_B
+	* 38_A
+	EOF
+	)
+'
+
+test_expect_success '--first-parent with filtered parents but both have childs' '
+	(
+	create_orphan _41 && test_commit 41_A && test_commit 41_B &&
+	create_orphan _42 && test_commit 42_A && test_commit 42_B &&
+	create_orphan _43 && test_commit 43_A && test_commit 43_B &&
+	git checkout _41 &&
+	TREE=$(git write-tree) &&
+	MERGE=$(git commit-tree $TREE -p _41 -p _42 -p _43 -m 41_octopus) &&
+	git reset --hard $MERGE &&
+	lib_test_check_graph --first-parent _41 _42 _43 <<-\EOF
+	* 41_octopus
+	| * 43_B
+	|  \
+	|   * 43_A
+	| * 42_B
+	| * 42_A
+	* 41_B
+	* 41_A
+	EOF
+	)
+'
+
+test_done

-- 
2.54.0

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Phillip Wood]

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

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Pablo Sabater]

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
>

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Phillip Wood]

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
>>

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Chandra Pratap]

Hi all,

On Fri, 15 May 2026 at 15:03, Phillip Wood <phillip.wood123@gmail.com> wrote:
>
> 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)

It probably *does* look clearer here, but I have the same reservations
against this as Junio: the break won't be as noticeable when --graph is
*not* used with --oneline.

> >>> 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

Right, this would be ideal but that would require too much change to the
existing graphing logic, and should be its own patch.

> > 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.

I guess that would make the examples look something like this:

  * A root
  * B root
  * C root
* D1 child
* D root

No cascading, and no need for that massive _ / collapse line.

* 10_A2
* 10_A1
 \
  * 10_A
*   10_M
| \ \
| | * 10_D
| * 10_C
* 10_B

I say it looks better than the alternatives, but I'm not sure if this will
be easy to implement. The diagonal connection line (\) will need to
be printed before printing the actual root commit, which will require
lookahead logic.

I'd prefer to avoid major surgery on the codebase.

> > 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
> >>
>

Thanks,
Chandra.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Pablo Sabater]

Hi Chandra, Phillip,

> > >
> > > 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.

Collapsing would be when branches move to the left, eg:

  *
  |\     <- merge
  | *
  |/     <- 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)
>
> It probably *does* look clearer here, but I have the same reservations
> against this as Junio: the break won't be as noticeable when --graph is
> *not* used with --oneline.
>
> > >>> 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
>
> Right, this would be ideal but that would require too much change to the
> existing graphing logic, and should be its own patch.

For the examples I'll use the term parentless instead of root, as
boundary commits are excluded even if they are roots.
By having is_parentless as a flag in 'git_graph' that every stage can
access we could modify the rendering and maybe completely drop the
commit placeholders, working on it for v4 but currently renders like
this

    * A parentless
      * B parentless
        * C parentless
  * D1 child
  * D parentless

(A has indentation when it could not have, but that would require a
lookahead if the next commit is also parentless)
But definitely a step forward.

Do we want cascading or just a fixed indentation?

    * A parentless
    * B parentless
    * C parentless
  * D1 child
  * D parentless

By being indented it indicates that it is parentless and that the one
below doesn't relate to it, but cascading looks clearer.

>
> > > 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.
>
> I guess that would make the examples look something like this:
>
>   * A root
>   * B root
>   * C root
> * D1 child
> * D root
>
> No cascading, and no need for that massive _ / collapse line.
>
> * 10_A2
> * 10_A1
>  \
>   * 10_A
> *   10_M
> | \ \
> | | * 10_D
> | * 10_C
> * 10_B
>
> I say it looks better than the alternatives, but I'm not sure if this will
> be easy to implement. The diagonal connection line (\) will need to
> be printed before printing the actual root commit, which will require
> lookahead logic.
>
> I'd prefer to avoid major surgery on the codebase.

Octopus merges need a pre-commit phase where an additional row
increases the space around a commit with multiple parents to make room
for it.
A new phase can be created similarly to pre-commit as pre-root where
the connection edge (\) can be printed before the indented commit.

So far this is the comparison:

indentation at root:

    * A parentless
  * B1 child
   \
    * B parentless
  * C1 child
  * C parentless

indentation AFTER the root (current v3):

  * A parentless
    * B1 child
   /
  * B parentless
    * C1 child
   /
  * C parentless

Karthik mentioned that by indenting the parentless, we lose the
consistency of having the roots on their real column and now some are
indented and some are not.
The biggest winner having the parentless indented are the merge commits:

  * A child
  * A child
    \
      * A parentless
  *-.   B child
  | \ \
  | |  * C parentless
  | * D parentless
  * E parentless

which IMO looks clearer than the commit shower:

  * A child
  * A child
  * A parentless
    *   B child
   /|\
  | | * C parentless
  | * D parentless
  * E parentless
>
>
> Thanks,
> Chandra.

Regards

--
Pablo

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

> By having is_parentless as a flag in 'git_graph' that every stage can
> access we could modify the rendering and maybe completely drop the
> commit placeholders, working on it for v4 but currently renders like
> this
>
>     * A parentless
>       * B parentless
>         * C parentless
>   * D1 child
>   * D parentless
>
> (A has indentation when it could not have, but that would require a
> lookahead if the next commit is also parentless)
> But definitely a step forward.
>
> Do we want cascading or just a fixed indentation?
>
>     * A parentless
>     * B parentless
>     * C parentless
>   * D1 child
>   * D parentless

I am late to the party, but I cannot get how the latter is viable.
If "A" had parent "B" whose parent was "C" that is root, wouldn't we
see the same output?  Or are we adding " parentless" at the end of
the one-liner log message?

The former, with the understanding that "two '*' commit marks
vertically adjacent have parent-child relationship, otherwise we
draw line between '*' to connect them if they have parent-child
relationship", does not have such a problem.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Pablo Sabater]

El mar, 19 may 2026 a las 2:03, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > By having is_parentless as a flag in 'git_graph' that every stage can
> > access we could modify the rendering and maybe completely drop the
> > commit placeholders, working on it for v4 but currently renders like
> > this
> >
> >     * A parentless
> >       * B parentless
> >         * C parentless
> >   * D1 child
> >   * D parentless
> >
> > (A has indentation when it could not have, but that would require a
> > lookahead if the next commit is also parentless)
> > But definitely a step forward.
> >
> > Do we want cascading or just a fixed indentation?
> >
> >     * A parentless
> >     * B parentless
> >     * C parentless
> >   * D1 child
> >   * D parentless
>
> I am late to the party, but I cannot get how the latter is viable.
> If "A" had parent "B" whose parent was "C" that is root, wouldn't we
> see the same output?  Or are we adding " parentless" at the end of
> the one-liner log message?

We wouldn't see the same output because A and B wouldn't get padded in
that case. Vertical adjacency between indented commits doesn't imply
relation because indentation means that they are "parentless",
ambiguity happens when there's no indentation, you can't know whether
they are related or not, but knowing that every indented commit is a
"parentless" eliminates the ambiguity.

* A child
* B child
 \
  * C parentless
* D1 child
* D parentless

Some different cases:

A child
 \
  B parentless
C parentless

  A parentless
  B parentless
C parentless

C has no indentation because if there's nothing to render below,
indentation is disabled.

  A parentless
B child
C parentless

Anyways, having more than 2 "parentless" commits one after the other
is strange. Cascading is just having a depth counter and printing the
padding depth times, so I'll keep it as it is more intuitive.
>
> The former, with the understanding that "two '*' commit marks
> vertically adjacent have parent-child relationship, otherwise we
> draw line between '*' to connect them if they have parent-child
> relationship", does not have such a problem.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Junio C Hamano]

Pablo Sabater <pabloosabaterr@gmail.com> writes:

>> > Do we want cascading or just a fixed indentation?
>> >
>> >     * A parentless
>> >     * B parentless
>> >     * C parentless
>> >   * D1 child
>> >   * D parentless
>>
>> I am late to the party, but I cannot get how the latter is viable.
>> If "A" had parent "B" whose parent was "C" that is root, wouldn't we
>> see the same output?  Or are we adding " parentless" at the end of
>> the one-liner log message?
>
> We wouldn't see the same output because A and B wouldn't get padded in
> that case. Vertical adjacency between indented commits doesn't imply
> relation because indentation means that they are "parentless",

Hmph, I guess such "the first column is special in that two commits
on consecutive lines with the asterisk on the same column, if only
that is on the first column, are parent-child, but it does not hold
in all other columns" was beyond my imagination. And that was why I
said I am late to the party.  Do others find such a rule intuitive?
I didn't (and that is what led me to ask the question).

> Anyways, having more than 2 "parentless" commits one after the other
> is strange. Cascading is just having a depth counter and printing the
> padding depth times, so I'll keep it as it is more intuitive.

Is everbody happy with this version, or will we see an updated final
reroll to tie any loose ends?  For example, do we need the above
"vertically adjacent commits are in parent-child relationship only
when they appear on the first column" given as a new instruction in
the documentation to help users read and understand what the graph
output is trying to tell them?

Thanks.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Pablo Sabater]

El mié, 10 jun 2026 a las 17:21, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> >> > Do we want cascading or just a fixed indentation?
> >> >
> >> >     * A parentless
> >> >     * B parentless
> >> >     * C parentless
> >> >   * D1 child
> >> >   * D parentless
> >>
> >> I am late to the party, but I cannot get how the latter is viable.
> >> If "A" had parent "B" whose parent was "C" that is root, wouldn't we
> >> see the same output?  Or are we adding " parentless" at the end of
> >> the one-liner log message?
> >
> > We wouldn't see the same output because A and B wouldn't get padded in
> > that case. Vertical adjacency between indented commits doesn't imply
> > relation because indentation means that they are "parentless",
>
> Hmph, I guess such "the first column is special in that two commits
> on consecutive lines with the asterisk on the same column, if only
> that is on the first column, are parent-child, but it does not hold
> in all other columns" was beyond my imagination. And that was why I
> said I am late to the party.  Do others find such a rule intuitive?
> I didn't (and that is what led me to ask the question).
>
> > Anyways, having more than 2 "parentless" commits one after the other
> > is strange. Cascading is just having a depth counter and printing the
> > padding depth times, so I'll keep it as it is more intuitive.
>
> Is everbody happy with this version, or will we see an updated final
> reroll to tie any loose ends?  For example, do we need the above
> "vertically adjacent commits are in parent-child relationship only
> when they appear on the first column" given as a new instruction in
> the documentation to help users read and understand what the graph
> output is trying to tell them?
>
> Thanks.

Hi!
No, it is not ready yet, sorry, I have to send the next version but I
cannot get some tests to work, I should have it by this week.

Thanks,
Pablo.

Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

[Chandra Pratap]

On Mon, 18 May 2026 at 18:57, Pablo Sabater <pabloosabaterr@gmail.com> wrote:
>
> Hi Chandra, Phillip,
>
> > > >
> > > > 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.
>
> Collapsing would be when branches move to the left, eg:
>
>   *
>   |\     <- merge
>   | *
>   |/     <- 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)
> >
> > It probably *does* look clearer here, but I have the same reservations
> > against this as Junio: the break won't be as noticeable when --graph is
> > *not* used with --oneline.
> >
> > > >>> 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
> >
> > Right, this would be ideal but that would require too much change to the
> > existing graphing logic, and should be its own patch.
>
> For the examples I'll use the term parentless instead of root, as
> boundary commits are excluded even if they are roots.
> By having is_parentless as a flag in 'git_graph' that every stage can
> access we could modify the rendering and maybe completely drop the
> commit placeholders, working on it for v4 but currently renders like
> this
>
>     * A parentless
>       * B parentless
>         * C parentless
>   * D1 child
>   * D parentless
>
> (A has indentation when it could not have, but that would require a
> lookahead if the next commit is also parentless)
> But definitely a step forward.
>
> Do we want cascading or just a fixed indentation?
>
>     * A parentless
>     * B parentless
>     * C parentless
>   * D1 child
>   * D parentless
>
> By being indented it indicates that it is parentless and that the one
> below doesn't relate to it, but cascading looks clearer.

Agreed, let's keep the cascading.

> >
> > > > 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.
> >
> > I guess that would make the examples look something like this:
> >
> >   * A root
> >   * B root
> >   * C root
> > * D1 child
> > * D root
> >
> > No cascading, and no need for that massive _ / collapse line.
> >
> > * 10_A2
> > * 10_A1
> >  \
> >   * 10_A
> > *   10_M
> > | \ \
> > | | * 10_D
> > | * 10_C
> > * 10_B
> >
> > I say it looks better than the alternatives, but I'm not sure if this will
> > be easy to implement. The diagonal connection line (\) will need to
> > be printed before printing the actual root commit, which will require
> > lookahead logic.
> >
> > I'd prefer to avoid major surgery on the codebase.
>
> Octopus merges need a pre-commit phase where an additional row
> increases the space around a commit with multiple parents to make room
> for it.
> A new phase can be created similarly to pre-commit as pre-root where
> the connection edge (\) can be printed before the indented commit.
>
> So far this is the comparison:
>
> indentation at root:
>
>     * A parentless
>   * B1 child
>    \
>     * B parentless
>   * C1 child
>   * C parentless
>
> indentation AFTER the root (current v3):
>
>   * A parentless
>     * B1 child
>    /
>   * B parentless
>     * C1 child
>    /
>   * C parentless
>
> Karthik mentioned that by indenting the parentless, we lose the
> consistency of having the roots on their real column and now some are
> indented and some are not.
> The biggest winner having the parentless indented are the merge commits:
>
>   * A child
>   * A child
>     \
>       * A parentless
>   *-.   B child
>   | \ \
>   | |  * C parentless
>   | * D parentless
>   * E parentless
>
> which IMO looks clearer than the commit shower:
>
>   * A child
>   * A child
>   * A parentless
>     *   B child
>    /|\
>   | | * C parentless
>   | * D parentless
>   * E parentless

I guess we're deciding between cleaner root (parentless) commits and
cleaner merge commits. I favour merge commits because they appear
much more frequently in most codebases.

> >
> >
> > Thanks,
> > Chandra.
>
> Regards
>
> --
> Pablo