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

From: Pablo Sabater <pabloosabaterr@gmail.com>
Date: May 18, 2026, 13:26 UTC
Message-ID: <CAN5EUNQoKRqt3FGLmzRGpPU1nO5jCAogP8Wm9gBZXuPbMNbQAw@mail.gmail.com>
In-Reply-To: <CA+J6zkTGgeNuH0eusTy+t8LO3bjygSz4svJB=K4R5ASmBdd0uQ@mail.gmail.com>

Hi Chandra, Phillip,

Show 12 quoted lines

> > >
> > > 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.
> >
> > I'm afraid I don't understand this - what does it mean for the
> > indentation to collapse, or not collapse.

Collapsing would be when branches move to the left, eg:

  *
  |\     <- merge
  | *
  |/     <- collapse
  *

Show 88 quoted lines

> > Looking at the examples Junio
> > gave they look quite nice to me, though I'd find it clearer if
> >
> >
> >   | | *  12345678 2021-01-14 merge xxxxx@xxxx into the history
> >   | | |\
> >   | | | \
> >   | | *  \  23456789 2021-01-12 merge citest into the main history
> >   | | |\  * 5505e019c2 2014-07-09 initial xxxxxx@xxxx
> >   | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
> > Added defau
> >   | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
> > f7daf088)
> >
> > was rendered as
> >
> >
> >   | | *  12345678 2021-01-14 merge xxxxx@xxxx into the history
> >   | | |\
> >   | | | *  5505e019c2 2014-07-09 initial xxxxxx@xxxx
> >   | | *    23456789 2021-01-12 merge citest into the main history
> >   | | |\
> >   | | | *  3e658f4085 2019-09-10 (wiki/wip-citest, origin/wip-citest)
> > Added defau
> >   | | | *  ad148aafe6 2019-09-10 Added default CI/CD Jenkinsfile (from
> > f7daf088)
>
> It probably *does* look clearer here, but I have the same reservations
> against this as Junio: the break won't be as noticeable when --graph is
> *not* used with --oneline.
>
> > >>> 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 shouldn't be a blank line between D and D1 so the two
> > alternatives take up the same amount of vertical space, the main
> > difference being whether D1 appears next to D
> >
> >      * A root     * A root
> >                     * B root
> >      * B root         * C root
> >                         * D1 child
> >      * C root         _/
> >                     /
> >      * D1 child    /
> >      * D root     * D root
> >
> > Of course if the indentation was smarter it would take up less room and
> > look better than having blank lines
> >
> >      * A root
> >        * B root
> >          * C root
> >      * D1 child
> >      * D root
>
> Right, this would be ideal but that would require too much change to the
> existing graphing logic, and should be its own patch.

For the examples I'll use the term parentless instead of root, as boundary commits are excluded even if they are roots. By having is_parentless as a flag in 'git_graph' that every stage can access we could modify the rendering and maybe completely drop the commit placeholders, working on it for v4 but currently renders like this

    * A parentless
      * B parentless
        * C parentless
  * D1 child
  * D parentless

(A has indentation when it could not have, but that would require a lookahead if the next commit is also parentless) But definitely a step forward.

Do we want cascading or just a fixed indentation?

    * A parentless
    * B parentless
    * C parentless
  * D1 child
  * D parentless

By being indented it indicates that it is parentless and that the one below doesn't relate to it, but cascading looks clearer.

Show 47 quoted lines

>
> > > 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.
> >
> > Yes, that is a bit confusing. I think the thing I find confusing with
> > this approach is that we're treating the commit rendered below the root
> > commit specially, rather than treating the root commit itself specially.
> > To me it is the root commit that's the odd one out because it does not
> > have any parents, but we treat the commit that's rendered below as the
> > odd one by indenting it relative to its parents.
>
> I guess that would make the examples look something like this:
>
>   * A root
>   * B root
>   * C root
> * D1 child
> * D root
>
> No cascading, and no need for that massive _ / collapse line.
>
> * 10_A2
> * 10_A1
>  \
>   * 10_A
> *   10_M
> | \ \
> | | * 10_D
> | * 10_C
> * 10_B
>
> I say it looks better than the alternatives, but I'm not sure if this will
> be easy to implement. The diagonal connection line (\) will need to
> be printed before printing the actual root commit, which will require
> lookahead logic.
>
> I'd prefer to avoid major surgery on the codebase.

Octopus merges need a pre-commit phase where an additional row increases the space around a commit with multiple parents to make room for it. A new phase can be created similarly to pre-commit as pre-root where the connection edge (\) can be printed before the indented commit.

So far this is the comparison:

indentation at root:

    * A parentless
  * B1 child
   \
    * B parentless
  * C1 child
  * C parentless

indentation AFTER the root (current v3):

  * A parentless
    * B1 child
   /
  * B parentless
    * C1 child
   /
  * C parentless

Karthik mentioned that by indenting the parentless, we lose the consistency of having the roots on their real column and now some are indented and some are not. The biggest winner having the parentless indented are the merge commits:

  * A child
  * A child
    \
      * A parentless
  *-.   B child
  | \ \
  | |  * C parentless
  | * D parentless
  * E parentless

which IMO looks clearer than the commit shower:

  * A child
  * A child
  * A parentless
    *   B child
   /|\
  | | * C parentless
  | * D parentless
  * E parentless

>
>
> Thanks,
> Chandra.

Regards

-- Pablo