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

From: James Coglan <jcoglan@gmail.com>
Date: Oct 14, 2019, 15:49 UTC
Message-ID: <34315593-7284-86a7-fe45-6d830352e69b@gmail.com>
In-Reply-To: <20191013071500.GA30652@sigill.intra.peff.net>

On 13/10/2019 08:15, Jeff King wrote:

Show 50 quoted lines

> 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).

You're probably right about the non-default mode hiding bugs, but if you did want to do this, I believe the original rendering can be preserved by the following small switches:

- always set `merge_layout = 1`; this will prevent skewing to the left
- do not modify `edges_added` if the last parent fuses with its neighbor

I believe all the layout changes are driven by these values, so you should be able to set in such a way as to preserve the original rendering by ignoring the special cases that lead to them having different values.