Re: [PATCH][BZ 18934] hu_HU: Fix multiple sorting bugs.

[Egmont Koblinger <egmont at gmail dot com>]

Hi,

Friendly ping... what's going on with this one?

I was the guy making the last few changes to this locale (even an
unfortunate regression), and now I also add the most extensive
unittesting any locale has (protecting against such regressions now or
in the future), so without even looking at this patch I guess you
should be quite confident that the patch only makes things better, not
worse.

Would it help if I broke it down to like 4 or 5 small patches on top
of each other, and added the unittests in the last step?

thanks,
egmont


On Mon, Oct 26, 2015 at 4:24 PM, Egmont Koblinger <egmont@gmail.com> wrote:
> Hello,
>
> Friendly ping - could you please take a look at this patch (version 5)?
>
> Is there anything I can help you with?
>
> Thanks,
> egmont
>
> On Wed, Oct 14, 2015 at 12:36 AM, Egmont Koblinger <egmont@gmail.com> wrote:
>> Hi,
>>
>> Please use the patch I attach now to this mail, not to the previous
>> one. Sorry for the confusion!
>>
>> I checked the previous patch many times, yet I missed something that
>> I've just discovered after sending the previous mail. I forgot one of
>> the compound letters from the unittest.
>>
>> The only change from the previous patch is the addition of these few
>> more lines in the unittest, so it has an even better coverage. The
>> patch to the locale definiton is unchanged.
>>
>> I've re-run the test and of course it still passes :)
>>
>> Thanks,
>> egmont
>>
>> On Tue, Oct 13, 2015 at 11:56 PM, Egmont Koblinger <egmont@gmail.com> wrote:
>>> Hi,
>>>
>>> Could you please review and apply the attached patch?
>>>
>>> Recommended commit message body (feel free to edit as you please):
>>> -----
>>> Fix sorting of long consonants, a regression introduced by #13547. Fix
>>> inconsistencies in uppercase vs. lowercase sorting. Fix diacritic
>>> ordering. Fix ordering of foreign accents.
>>>
>>> Add an extensive test file.
>>>
>>>     [BZ #18934]
>>>     * locales/hu_HU: Fix multiple bugs.
>>>     * hu_HU.in: New file.
>>>     * Makefile (test-input): Add hu_HU.UTF-8.
>>> -----
>>>
>>> I know that generally one patch per issue is a cleaner approach, but
>>> this time apologize for an all-in-one: the patches would heavily
>>> conflict, and it would be really cumbersome to unittest an incremental
>>> series. Instead, think about it as TDD (test driven development): I
>>> attach a decent unittest with explanations and pointers to the rules,
>>> and a locale definition that implements them.
>>>
>>> The addressed bugs are:
>>>
>>> - The fix to bug 13547 was incorrect and introduced a regression. It
>>> fixed a corner case, whereas I didn't realize it broke a more typical
>>> once. See details over there.
>>>
>>> - Two minor bugs/inconsistencies wrt. sorting upper/lowercase values,
>>> as described in bug 18587.
>>>
>>> - Someone enabled backwards ordering of diacrits by default (bug
>>> 17750), breaking tons of locales including Hungarian. So disable
>>> backwards ordering in this locale definition.
>>>
>>> - Foreign accents should be sorted after the native Hungarian ones, it
>>> wasn't the case so far.
>>>
>>> Plus, a unittest is added which is far more extensive than any other
>>> locale has. It includes all the examples from the official rules of
>>> Hungarian orthography's corresponding sections, as well as thorough
>>> testing of all corner cases I could think of, created by me; and
>>> comments all around.
>>>
>>> In addition to fixing a(n unfortunately relatively unsignificant)
>>> locale, I hope that this unittest file will encourage other locale
>>> maintainers to create similarly extensive tests, increasing the
>>> quality of other locales in the long run and preventing regressions
>>> (such as the backward diacritics ordering) from sneaking in.
>>>
>>> Thanks a lot,
>>> egmont