Re: [PATCH v2 1/2] xdiff: implement a zealous diff3, or "zdiff3"

[Phillip Wood <phillip.wood123@gmail.com>]

On 18/09/2021 23:06, Elijah Newren wrote:
> 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

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