Thread (114 messages) 114 messages, 7 authors, 2d ago

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help