From: Derrick Stolee <stolee@gmail.com>
To: Jeff King <peff@peff.net>
Cc: Bradley Smith <brad@brad-smith.co.uk>,
Junio C Hamano <gitster@pobox.com>,
James Coglan <jcoglan@gmail.com>,
git@vger.kernel.org
Subject: Re: Assertion in git log graphing [regression in v2.25]
Date: Tue, 7 Jan 2020 09:22:34 -0500 [thread overview]
Message-ID: <39277f9a-f88d-94a3-2a8b-b6e0a3dfdc62@gmail.com> (raw)
In-Reply-To: <20200107140417.GA12242@coredump.intra.peff.net>
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
next prev parent reply other threads:[~2020-01-07 14:22 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-07 11:24 Assertion in git log graphing Bradley Smith
2020-01-07 11:48 ` Assertion in git log graphing [regression in v2.25] Jeff King
2020-01-07 12:22 ` Derrick Stolee
2020-01-07 12:42 ` Derrick Stolee
2020-01-07 12:50 ` Eric Sunshine
2020-01-07 12:56 ` Derrick Stolee
2020-01-07 13:14 ` Eric Sunshine
2020-01-07 13:25 ` Derrick Stolee
2020-01-07 14:04 ` Jeff King
2020-01-07 14:22 ` Derrick Stolee [this message]
2020-01-07 14:43 ` Derrick Stolee
2020-01-07 14:57 ` Assertion in git log graphing Derrick Stolee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=39277f9a-f88d-94a3-2a8b-b6e0a3dfdc62@gmail.com \
--to=stolee@gmail.com \
--cc=brad@brad-smith.co.uk \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=jcoglan@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).