From: Pablo <pabloosabaterr@gmail.com>
Date: Fri, 03 Apr 2026 08:25:57 GMT
Subject: Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root
Message-ID: <CAN5EUNQZLHDSyLB=Z6RarfD1re3d=+tsUHCrL6QrjjU7eObRSQ@mail.gmail.com>
In-Reply-To: <xmqqpl4gocrj.fsf@gitster.g>

El vie, 3 abr 2026 a las 7:04, Junio C Hamano (<gitster@pobox.com>) escribió:
>
> Pablo Sabater <pabloosabaterr@gmail.com> writes:
>
> > This issue was reported by Junio at:
> >   https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
>
> You are giving me too much credit.  I just knew about previous
> attempts and the gotchas.

Should I mention that thread instead ?

>
> One thing that we may want to make sure your solution gets right is
> the issue depicated in two graphs in the footnote of this message:
>
>   https://lore.kernel.org/git/xmqqwnwajbuj.fsf@gitster.c.googlers.com/
>
>     Stepping back a bit, I think concentrating too much on "is it
>     root?" is a wrong way to think about the problem.  Suppose you
>     have two histories, e.g. (time flows from left to right; A and X
>     are roots)
>
>             A---B
>                  \
>           X---Y---Z
>
>     and doing "git log --graph --oneline Z" would show A, B, X, Y
>     and Z.
>
>     But in a slightly modified graph:
>
>           C
>          /
>         O---A---B
>                  \
>           X---Y---Z
>
>     if you do "git log --graph --oneline C..Z", you should see the
>     same commits listed as above (A, B, X, Y and Z), and most likely
>     in the same order.

I can't find the issue with the graph above, it would be shown:

            A---B
                 \
          X---Y---Z

but we shouldn't want indentation here tho

  *   Z
  |\
  | * B
  | * A
  * Y
  * X

B is the parent of A and there is no commit on a third branch that
could try to get below A.

But I do find the issue with focusing on: is a root ? for example with
this graph:

  O---A

  X---Y

If we O..A Y, it shows A, Y, X but because I only look for roots it
ends up looking like:

  * A <- not a root but O is excluded
  * Y <- no indentation
  * X

Then it's more something like 'seems_root' rather than 'is_root', and
it should look like

  * A
    * Y
   /
  * X

This would make on your last graph to have indentation at the right of
Y, below A, even if not needed, it would protect A's spot the row
below, which I think is desirable and what you meant with the
examples.

>
> The way we draw A and make sure one raw below A in the same lane is
> vacant (to avoid something that is not an ancestor of A steals that
> spot) is applicable to both graphs.  The reason why we try to keep
> one row below A vacant is not because it is a root, but because in
> the graph being drawn, none of A's parent will appear.  Obviously if
> A is root, none of A's parent will appear in the graph, but in the
> latter topology where we are drawing C..Z, none of A's parent will
> appear not because A is root, but because all the parents of A is
> excluded.
>

Thanks for the feedback
Pablo