Re: Assertion in git log graphing [regression in v2.25]

From: Derrick Stolee <stolee@gmail.com>
Date: Jan 7, 2020, 14:22 UTC
Message-ID: <39277f9a-f88d-94a3-2a8b-b6e0a3dfdc62@gmail.com>
In-Reply-To: <20200107140417.GA12242@coredump.intra.peff.net>

On 1/7/2020 9:04 AM, Jeff King wrote:

Show 36 quoted lines

> On Tue, Jan 07, 2020 at 08:25:59AM -0500, Derrick Stolee wrote:
> 
>> On 1/7/2020 6:48 AM, Jeff King wrote:
>>> The assertion itself is quite old, so I wondered if it was even still
>>> relevant. Removing it does produce a reasonable-looking graph:
>>
>> As I'm digging into this case, and finding when the assertion is hit,
>> I see that the issue is in the line further below your coloring issue:
> 
> Oh, you're right. I totally missed that.
> 
> So perhaps we have two bugs, or perhaps they have the same root cause.
> 
>>>   | | | | * dd068b4 Merge commit '8f076d8' into HEAD
>>>   | |_|_|/| 
>>>   |/| | |/  
>>>   | | |/|   
>>>   | |/| |   
>>>   | * | | 8f076d8 5
>>
>> What is output is actually this, above. But the logic that includes the
>> assert is checking where the underscores end, and the shown underscores
>> actually pass the check. The issue is that it seems like it really wants
>> to show this:
>>
>>>   | | | | * dd068b4 Merge commit '8f076d8' into HEAD
>>>   | |_|_|/| 
>>>   |/| |_|/  
>>>   | |/| |   
>>>   | * | | 8f076d8 5
>>
>> Note that I dropped a line and compressed a slash into an underscore. It's
>> on that line that this condition is being hit.
> 
> Hrm. I could see either being acceptable, but I do think the second one
> is a bit easier to read. I'm not sure which was intended for this case.

Somewhat expanding the situation to test more of these collapses, I can get the following graph:

*   6_M1
|\
| | *   6_M2
| | |\
| | | * 6_H
| | | | *   6_M3
| | | | |\
| | | | | * 6_J
| | | | * | 6_I
| | | | | | *   6_M4
| |_|_|_|_|/|\
|/| | | | |/ /
| | | | |/| /
| | | |/| |/
| | |/| |/|
| |/| |/| |
| | |/| | |
| | * | | | 6_G
| | | |_|/
| | |/| |
| | * | | 6_F
| * | | | 6_E
| | |/ /
| |/| |
| * | | 6_D
| | |/
| |/|
* | | 6_C
| |/
|/|
* | 6_B
|/
* 6_A

Note how 6_M4 has three parents, and the first parent has a horizontal edge. Even giving an extra padding line between horizontal edges, we _could_ output the following instead:

| | | | | | *   6_M4
| |_|_|_|_|/|\
|/| | | | |/ /
| | |_|_|/| /
| |/| | | |/
| | | |_|/|
| | |/| | |
| | * | | | 6_G

However, I'll stick with the fix for the coloring issue. I think it was a previous bug that just wasn't hit until now.

Thanks, -Stolee