On 12/10/2019 02:36, Derrick Stolee wrote: > On 10/11/2019 12:50 PM, James Coglan wrote: >> On 10/10/2019 18:19, Derrick Stolee wrote: >>> On 10/10/2019 12:13 PM, James Coglan via GitGitGadget wrote: >>>> +++ b/t/t4215-log-skewed-merges.sh >>>> @@ -0,0 +1,42 @@ >>>> +#!/bin/sh >>>> + >>>> +test_description='git log --graph of skewed merges' >>>> + >>>> +. ./test-lib.sh >>>> + >>>> +test_expect_success 'setup left-skewed merge' ' >>> >>> >>> Could you skew this example to include a left-skewed octopus merge >>> (and use fewer Git processes) with the following: >>> >>> git checkout --orphan _a && test_commit A && >>> git switch -c _b _a && test_commit B && >>> git switch -c _c _a && test_commit C && >>> git switch -c _d _a && test_commit D && git switch -c _e _b && git >>> merge --no-ff _c _d E && >>> git switch -c _f _a && git merge --no-ff _d -m F && git checkout _a >>> && git merge --no-ff _b _c _e _f -m G >>> and I think the resulting output will be: >>> >>> *-----. G >>> |\ \ \ \ >>> | | | | * F >>> | |_|_|/| >>> |/| | | | >>> | | | * | E >>> | |_|/|\| >>> |/| | | | >>> | | |/ * D >>> | |_|__/ >>> |/| | >>> | | * C >>> | |/ >>> |/| >>> | * B >>> |/ >>> * A >> >> At this point in the history, commit E won't render like that -- this is >> before the change that flattens edges that fuse with the merge's last >> parent. I think the display of this history at this point will be: >> >> *-----. G >> |\ \ \ \ >> | | | | * F >> | |_|_|/| >> |/| | | | >> | | | * | E >> | |_|/|\ \ >> |/| |/ / / >> | | | | / >> | | | |/ >> | | | * D >> | |_|/ >> |/| | >> | | * C >> | |/ >> |/| >> | * B >> |/ >> * A >> >> Is there a particular reason for wanting to include this test case? What >> particular combination of states is it designed to test? (My guess is that >> it includes an octopus merge where the original test does not.) I'd be happy >> to add it at the appropriate point in the history if it's adding coverage >> not provided by the other tests. > > Thanks for correcting my test case. It also helps that you would show the > change in behavior in your later commits. > > My reason to include this test is because it includes a regular merge and an > octopus merge, both of which have a skewed render. Many times logic that > applies to a normal merge breaks with octopus merges, so I try to include > them whenever possible.
Thanks, I've now incorporated your suggested test into my patch. I had to amend it slightly as it turns out the above history is not valid; G is not a possible merge because one of its parents (C) is an ancestor of another (E). The actual example I've added is this: *-----. 0_H |\ \ \ \ | | | | * 0_G | |_|_|/| |/| | | | | | | * \ 0_F | |_|/|\ \ |/| | | |/ | | | | * 0_E | |_|_|/ |/| | | | | * | 0_D | | |/ | | * 0_C | |/ |/| | * 0_B |/ * 0_A I've also added another commit before beginning this work that adds the example from the cover letter, so you can see it changing with each commit, namely this history: * H |\ | * G | |\ | | * F | | | | | \ | *-. \ E | |\ \ \ |/ / / / | | | / | | |/ | | * D | * | C | |/ * | B |/ * A