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

[Derrick Stolee <stolee@gmail.com>]

On 1/7/2020 9:04 AM, Jeff King wrote:
> 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