Re: [PATCH v9 0/4] graph: indent visual roots in graph
- From
Pablo Sabater <pabloosabaterr@gmail.com>
- Date
- Jul 12, 2026, 16:59 UTC
- Message-ID
- <DJWR4GEV14P4.3G9N0ZL1R8VDL@gmail.com>
- In-Reply-To
- <alOOXKGIB8BqACxR@exploit>
On Sun Jul 12, 2026 at 3:10 PM CEST, Mirko Faina wrote:
Show 81 quoted lines
> On Sun, Jul 12, 2026 at 11:26:27AM +0530, Chandra Pratap wrote: >> 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. > >> > > 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
FNo 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
EWhich 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
EWhich I find the pyramid effect uncomfortable. What about capping at 4 columns?
1.
A
B
C
D
E
F
G
H2.
A
B
C
D
E
F
FI 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