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

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

On Tue, Jan 07, 2020 at 08:25:59AM -0500, Derrick Stolee wrote:

quoted

On 1/7/2020 6:48 AM, Jeff King wrote:

quoted

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.

quoted

quoted

  | | | | * 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:

quoted

  | | | | * 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