Subject: Re: [PATCH 00/11] Improve the readability of log --graph output

From: Jeff King <[email-masked]>
Date: Sun, 13 Oct 2019 03:15:01 -0400

On Thu, Oct 10, 2019 at 09:13:42AM -0700, James Coglan via GitGitGadget wrote:

> A final addition to that set of changes fixes the coloring of dashes that
> are drawn next to octopus merges, in a manner compatible with all these
> changes. The early commits in this set are refactorings that make the
> functional changes easier to introduce.

As somebody who has pondered the octopus coloring code (for an
embarrassingly long time considering that it still has some bugs!), let
me just say thank you for taking this on. :)

Moreover, I'll echo Dscho's comments elsewhere on the quality of this
series. It's a tricky topic to explain, and the way you've broken it up,
along with the commit messages, comments, and diagrams made it much
easier to follow.

Others have already commented on things I saw while reading it, so I'll
just add a few more thoughts.

> This series of patches are designed to improve the output of the log --graph
> command; their effect can be summed up in the following diagram:
> 
>     Before                    After
>     ------                    -----
> 
>     *
>     |\
>     | *                       *
>     | |\                      |\
>     | | *                     | *
>     | | |                     | |\
>     | |  \                    | | *
>     | *-. \                   | * |
>     | |\ \ \                  |/|\|
>     |/ / / /                  | | *
>     | | | /                   | * |
>     | | |/                    | |/
>     | | *                     * /
>     | * |                     |/
>     | |/                      *
>     * |
>     |/
>     *

I wondered if anybody would prefer the sparseness of the "before"
diagram, and if that would merit having two modes that could selected at
runtime. I'm not sure I'd want to carry the code for both types, though;
it seems like a recipe for the non-default output format to accrue a
bunch of bugs (since the graph code has proven itself to be a magnet for
off-by-ones and other weirdness).

Diffing the output of "git diff --color --graph --oneline" on git.git
both before and after your patch, the changes all look generally
positive to me. The graph above is pretty dense, but that's not really
what real-world graphs look like; it was designed to show off the
changes.

That plus the fact that you're fixing real bugs (like the octopus
coloring) makes me inclined to just move to your suggested output.

-Peff