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

On Sun, 14 Jun 2026 at 00:39, Pablo Sabater [off-list ref] wrote:

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.

It's slightly disappointing that we couldn't find a way to fix this
after all, but at least the bug is non-breaking and the added
NEEDSWORK properly documents the issue for someone else
to tackle in the future.

Other than that, this version looks fine to me.

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

V4 DIFF:

- Fixed test to be shown as expected by unsetting COMMIT_GRAPH

Signed-off-by: Pablo Sabater <redacted>
---
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           | 467 +++++++++++++++++++++++++++++
 t/t6016-rev-list-graph-simplify-history.sh |  25 +-
 6 files changed, 759 insertions(+), 34 deletions(-)
---
base-commit: 3e65291872de10c3f0bf05ea8c24187e7a71ebf0
change-id: 20260612-ps-pre-commit-indent-39ca72816382

Best regards,
--
Pablo Sabater [off-list ref]