From: Pablo Sabater Date: Fri, 10 Jul 2026 20:29:44 GMT Subject: Re: [PATCH v8 4/4] graph: indent visual root in graph Message-ID: In-Reply-To: El vie, 10 jul 2026 a las 20:07, Mirko Faina () escribió: > > On Fri, Jul 10, 2026 at 12:37:07PM +0200, Pablo Sabater 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 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. > > > > [1]: https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/ > > > > Helped-by: Kristofer Karlsson > > Mentored-by: Karthik Nayak > > Mentored-by: Chandra Pratap > > Signed-off-by: Pablo Sabater > > --- > > graph.c | 235 +++++++++++++++++++ > > t/meson.build | 1 + > > t/t4218-log-graph-indentation.sh | 473 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 709 insertions(+) > > This doesn't seem to work for every visual root e.g. > > git log --graph --oneline --author="Mirko Faina" > > The visual roots are not indented. > > > +/* > > + * 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); > > +} > > I suspect this behaviour is due to these assumptions being too strict. > > When we use the --author option the parents are not filtered out, so it > doesn't return NULL desipte being a visual root. We realize it is a > visual root only on the next commit, but once we are on the next commit > we can't indent as we have already printed this commit. > > We realize only on the next commit after hitting simplify_commit(), it > calls get_commit_action() and checks if should keep the commit based on > the regex we provided. If the regex is not matched the commit is just > ignored (we do not filter parents based on regex when we expand a topo > walk). > > At least that's what I gather, if anyone can confirm this... Hi! Yes, I just tried with the same: git log --graph --oneline --author="Mirko Faina" And no indentation sadly, if when we use --author the parents are not excluded then the c->parents is not enough. I think that it should be fine if we iterate each parent and call graph_is_interesting() that also calls get_commit_action() as a fallback. Something like: graph_is_visual_root_candidate(): /* We keep ignoring boundary commits */ if (c->object.flags & BOUNDARY) return 0; /* Check the parents if they are not excluded because of options like --author */ for (p = c->parents; p; p = p->next) if(graph_is_interesting(graph, p->item)) return 0; return 1; I haven't tried yet though, but if --author has this problem, probably other options like --grep would likely fail too because of the same reason. Thanks for the feedback, Pablo