Re: [PATCH v9 0/4] graph: indent visual roots in graph
From: Pablo Sabater <hidden>
Date: 2026-07-12 17:00:01
On Sun Jul 12, 2026 at 3:10 PM CEST, Mirko Faina wrote:
On Sun, Jul 12, 2026 at 11:26:27AM +0530, Chandra Pratap wrote:quoted
Tying graph-drawing logic to specific formatting flags could introduce inconsistencies. For example, if a user relies on a custom format like --format="%h %s", the output is functionally single-line and suffers from the exact same ambiguity, but it would miss the fix. Even in multi-line formats, relying on the absence of a '|' character to spot unrelated commits requires active effort. Indentation provides an immediate visual cue that breaks the vertical lineage, which is helpful regardless of the commit message length. I agree with Pablo: for users who strictly want the old behavior, an opt-out flag keeps the graph logic decoupled from the formatting logic.In that case, together with --[no]-graph-indent, a configuration variable like "graph.indent" could be introduced to reduce the usage of --[no]-graph-indent for those that would like to retain the old behaviour for most formats.quoted
quoted
quoted
Apart from having an option to disable indentation. We could have the cascading to have a limit or make it zig-zag: instead of: A B C D We could do: A B C D This would have its own edge cases like: A B C <- if we zig-zag here C and D become ambiguous, currently we are D indenting only the last commits (visual roots) here we would have D to chose between continuing cascading or indenting the first of D. I'm not so sure if I like the zig-zag solution because we need to think again if it causes an ambiguity, but I wanted to mention it. I think we need some more opinions about the design.I don't dislike the the current solution but I can see it degenerating if someone contributes a lot of one-patch series. Maybe you could indent commits that are both head and tail up to two levels and then on the third go back to the beginning of the line. That way you kind of have a zig-zag but without ambiguity. You'd only have to add a counter to keep track of the level of indentation.Not sure about this. A zig-zag pattern visually mimics branching and merging, which makes unrelated commits look like a complex merge topology. I also have a feeling that this will end up recreating the exact ambiguity this patch series is trying to fix.While a zig-zag pattern might be ambiguous, what I proposed is a little different. What I proposed is effectively a wrapping for anything that goes beyond two levels of indentation. I don't think it would look anything like a fork/merge pattern. * A * B * C * D * E * F The difference between two indentation levels and no indentation is very noticeble, I don't think anyone confused this. This would fix the staircase pattern on adjacent one-patch series.
I agree that having an infinite stair is not a good solution. the 3
column wrap looks reasonable.
I see two cases with this wrap:
1. No conflict case:
A
B
C
D
E
F
No ambiguity, this would be the ideal case.
2. Ambiguity:
If it happens that the visual number on visual roots meet the condition
(number_of_visual_roots % 3 == 0) and the next commit is NOT a visual
root this would happen:
A
B
C
D
E
E
Which would be ambiguous. The solution is to check with the lookahead
buffer that we have since patch 3 if the next is a visual root, if it's
not we indent D anyway:
A
B
C
D
E
E
Which I find the pyramid effect uncomfortable.
What about capping at 4 columns?
1.
A
B
C
D
E
F
G
H
2.
A
B
C
D
E
F
F
I prefer the 4 column wrap because it looks more abrupt and IMO shows
better that the commits are unrelated.
What do you think?
Also, about the no-opt option "--no-graph-indent" is still wanted
regardless of the final design that we choose?
Thanks,
Pablo