Re: [PATCH v2 1/2] xdiff: implement a zealous diff3, or "zdiff3"
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Sep 24, 2021, 10:09 UTC
- Message-ID
- <69635d07-7b4e-e94e-db98-0b8f4ea566d7@gmail.com>
- In-Reply-To
- <CABPp-BHXUNTuYPdCzfKhkvr23W3PODzti84bed6uEP5q+sj0TQ@mail.gmail.com>
On 18/09/2021 23:06, Elijah Newren wrote:
Show 50 quoted lines
> On Wed, Sep 15, 2021 at 4:22 AM Phillip Wood <phillip.wood123@gmail.com> wrote:
>>
>> On 15/09/2021 11:25, Phillip Wood wrote:
>>> I do wonder (though a brief try failed to trigger it) if there are cases
>>> where the diff algorithm does something "clever" which means it does not
>>> treat a common prefix or suffix as unchanged (see d2f82950a9
>>> ("Re(-re)*fix trim_common_tail()", 2007-12-20) for a related issue). We
>>> could just trim the common prefix and suffix from the two sides
>>> ourselves using xdl_recmatch().
>>
>> Here is an evil test case that shows this problem (diff on top of your patch)
>>
>>
>> diff --git a/t/t6427-diff3-conflict-markers.sh b/t/t6427-diff3-conflict-markers.sh
>> index de9c6190b9..836843c6b0 100755
>> --- a/t/t6427-diff3-conflict-markers.sh
>> +++ b/t/t6427-diff3-conflict-markers.sh
>> @@ -219,8 +219,9 @@ test_setup_zdiff3 () {
>> test_write_lines 1 2 3 4 5 6 7 8 9 >basic &&
>> test_write_lines 1 2 3 AA 4 5 BB 6 7 8 >middle-common &&
>> test_write_lines 1 2 3 4 5 6 7 8 9 >interesting &&
>> + test_write_lines 1 2 3 4 5 6 7 8 9 >evil &&
>>
>> - git add basic middle-common &&
>> + git add basic middle-common interesting evil &&
>> git commit -m base &&
>>
>> git branch left &&
>> @@ -230,19 +231,21 @@ test_setup_zdiff3 () {
>> test_write_lines 1 2 3 4 A B C D E 7 8 9 >basic &&
>> test_write_lines 1 2 3 CC 4 5 DD 6 7 8 >middle-common &&
>> test_write_lines 1 2 3 4 A B C D E F G H I J 7 8 9 >interesting &&
>> + test_write_lines 1 2 3 4 X A B C 7 8 9 >evil &&
>> git add -u &&
>> git commit -m letters &&
>>
>> git checkout right &&
>> test_write_lines 1 2 3 4 A X C Y E 7 8 9 >basic &&
>> test_write_lines 1 2 3 EE 4 5 FF 6 7 8 >middle-common &&
>> test_write_lines 1 2 3 4 A B C 5 6 G H I J 7 8 9 >interesting &&
>> + test_write_lines 1 2 3 4 Y A B C B C 7 8 9 >evil &&
>> git add -u &&
>> git commit -m permuted
>> )
>> }
>>
>> -test_expect_failure 'check zdiff3 markers' '
>> +test_expect_success 'check zdiff3 markers' '
>
> ...except your new testcase makes it fail.Sorry I meant to remove that, I'd changed it to test_expect_success so I could poke about in the test repo when it failed.
Best Wishes
Phillip
Show 22 quoted lines
> >> test_setup_zdiff3 && >> ( >> cd zdiff3 && >> @@ -251,6 +254,14 @@ test_expect_failure 'check zdiff3 markers' ' >> >> test_must_fail git -c merge.conflictstyle=zdiff3 merge -s recursive right^0 && >> >> + test_write_lines \ >> + 1 2 3 4 \ >> + "<<<<<<< HEAD" X A \ >> + "||||||| $(git rev-parse --short HEAD^1)" 5 6 ======= \ >> + Y A B C ">>>>>>> right^0" \ >> + B C 7 8 9 >expect && >> + test_cmp expect evil && >> + > > Yeah, this is an interesting testcase, and I agree with the 'expect' > choice you wrote, but the current code doesn't produce it. I'll > update the patches and send out another round, and then do you want to > try your xdl_recmatch() magic to fix this testcase? >