Re: [GSoC RFC PATCH 0/1] graph: add indentation for commits preceded by a root

From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: May 14, 2026, 17:45 UTC
Message-ID: <CAN5EUNQCsKD0CJqDi43i2JVBQQChAZVt_THQ1wGpdeydNHHCFw@mail.gmail.com>
In-Reply-To: <26d887d2-6ec2-4af1-b0bd-8e9b017bb4dd@gmail.com>

El jue, 14 may 2026 a las 17:15, Phillip Wood (<phillip.wood123@gmail.com>) escribió:

Show 29 quoted lines

>
> Hi Pablo
>
> On 02/04/2026 22:17, Pablo Sabater wrote:
> > When having a history with multiple root commits and drawing the history
> > near the roots, the graphing engine renders the commit one below the other,
> > seeming that they are related, which makes the graph confusing.
> >
> > This issue was reported by Junio at:
> >    https://lore.kernel.org/git/xmqqikaawrpx.fsf@gitster.g/
> >
> > e.g.:
> >
> >    * root-B
> >    * child-A2
> >    * child-A1
> >    * root-A
> >
> > [...]
>  >
> >    * root-B
> >      * child-A2
> >     /
> >    * child-A1
> >    * root-A
>
> I'm rather late to the party here, but personally I find the indentation
> a bit confusing, it would be clearer to me if we had a blank line after
> a root commit

Hi,

Show 9 quoted lines

>
>      * root-B
>
>      * child-A2
>      * child-A1
>      * root-A
>
> It takes the same amount of vertical space but keeps the children of
> root-A together.

I have mixed feelings about which approach to choose.
The idea of a blank line was thought at
https://lore.kernel.org/git/xmqq8s8vvw9m.fsf@gitster.c.googlers.com/
but Junio argued against it for having an extra row because the
indentation he proposed didn't collapse, however I find indentation +
no collapse the most confusing one.
I'd say that I'm fine with both approaches, blank line or indentation
+ collapse.

Show 18 quoted lines

> > without the patch:
> >
> >    * A root
> >    * B root
> >    * C root
> >    * D1 child
> >    * D root
> >
> > with the patch, the indentation cascades:
> >
> >    * A root
> >      * B root
> >        * C root
> >          * D1 child
> >       _ /
> >      /
> >     /
> >    * D root

  * A root

  * B root

  * C root

  * D1 child

  * D root

Here I think a blank line looks worse, too much space for just 5 commits and becomes one extra line which if this were like up to 7 or more parentless commits one after the other would be more noticeable. But there are cases that blank line might be better:

  * 10_A2
  * 10_A1
  * 10_A
    *   10_M
   /|\
  | | * 10_D
  | * 10_C
  * 10_B

Feels like a shower of commits instead of an indented merge.

Pro to the blank line, the parentless check is the same and it's just printing a '\n' at the right spot, while indent i'm mimicking like if there was a commit there. Anyways, I think in the majority of the cases the indentation + collapsing looks better. Sorry for the brief reply, I'm busy today.

Regards,

-- Pablo

Show 64 quoted lines

>
> Thanks
>
> Phillip
>
> > This is done by adding a is_placeholder flag to the columns, the root commit
> > is actually there but marked as a placeholder
> >
> > e.g.:
> >
> >     * root-B
> >    (B) * child-A2
> >      /
> >     * child-A1
> >     * root-A
> >
> > (B) would be root-B column with the placeholder flag active.
> >
> > Then teaching the rendering function to print a padding ' ' when meeting a
> > placeholder column outputs the second example.
> >
> > There could also be the case where there are multiple roots
> >
> > without the patch:
> >
> >    * A root
> >    * B root
> >    * C root
> >    * D1 child
> >    * D root
> >
> > with the patch, the indentation cascades:
> >
> >    * A root
> >      * B root
> >        * C root
> >          * D1 child
> >       _ /
> >      /
> >     /
> >    * D root
> >
> > the _ / might look weird but that's how the collapsing rendering does it
> > for big gaps, this case being from the 4th column to the 0th column.
> > Another patch could change the collapsing rendering for placeholders ?
> > I haven't done it to keep it minimal, but a follow up could make it
> > to be straight '/'. This would make it bigger but easier for the eye to follow.
> > IMO is not worth it, but opinions are welcome.
> >
> > The patch also adds tests for different cases like a root preceding multiple
> > parents merges and the examples above.
> >
> > There could be some edge cases still so any testing is very welcome.
> >
> > Pablo Sabater (1):
> >    graph: add indentation for commits preceded by a root
> >
> >   graph.c                      |  68 ++++++++++++++++--
> >   t/t4215-log-skewed-merges.sh | 136 +++++++++++++++++++++++++++++++++++
> >   2 files changed, 198 insertions(+), 6 deletions(-)
> >
> >
> > base-commit: 256554692df0685b45e60778b08802b720880c50
>