<i18n dev> RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Joe Wang huizhe.wang at oracle.com
Tue Aug 18 21:13:13 UTC 2020


Hi Naoto,

That's nice!  The change looks good to me.

Regards,
Joe

On 8/18/20 11:37 AM, naoto.sato at oracle.com wrote:
> Hi Joe,
>
> Thank you for your comment. I consolidated those duplicated pieces 
> into one piece. Did not make it a private method, though, as it would 
> need to return two results from the method (cnfMultiplier and whether 
> to return immediately for no-placeholder cases).
>
> https://cr.openjdk.java.net/~naoto/8251499/webrev.02/
>
> Naoto
>
> On 8/17/20 11:52 PM, Joe Wang wrote:
>> Hi Naoto,
>>
>> Looks good overall. One nit, blocks 1633-1639 and 1642-1649 may share 
>> a common private method with a parameter that takes either 
>> matchedPosIndex or matchedNegIndex, if you want.
>>
>> Best,
>> Joe
>>
>> On 8/17/20 4:42 PM, naoto.sato at oracle.com wrote:
>>> Hi Joe,
>>>
>>> It turned out that the previous fix did not address plural format 
>>> cases. That means that just making the divisor negative to indicate 
>>> non-placeholder cannot distinguish multiple plural cases with the 
>>> same divisor. Instead, I created a list of placeholders (minimum 
>>> digits) for each index and count. Here is the updated webrev:
>>>
>>> http://cr.openjdk.java.net/~naoto/8251499/webrev.01/
>>>
>>> I added a new test case (COMPACT_PATTERN14), which actually is 
>>> extracted from CLDR 38 Somali locale that demonstrates the issue. 
>>> I'd appreciate your further review.
>>>
>>> Naoto
>>>
>>> On 8/14/20 6:21 PM, Joe Wang wrote:
>>>> Hi Naoto,
>>>>
>>>> Looks good to me.
>>>>
>>>> While a negative divisor representing no zeros is newly introduced, 
>>>> the "divisor > 0" checks seem to have always been beneficial.  I 
>>>> had to count the number of ""s in COMPACT_PATTERN13 :-)
>>>>
>>>> Have a great weekend!
>>>> Joe
>>>>
>>>> On 8/14/2020 3:20 PM, naoto.sato at oracle.com wrote:
>>>>> Hello,
>>>>>
>>>>> Please review the fix for the following issue:
>>>>>
>>>>> https://bugs.openjdk.java.net/browse/JDK-8251499
>>>>>
>>>>> The proposed changeset is located at:
>>>>>
>>>>> https://cr.openjdk.java.net/~naoto/8251499/webrev.00/
>>>>>
>>>>> The current implementation of CompactNumberFormat assumes that 
>>>>> there is always the number placeholder part in compact patterns. 
>>>>> This is not always true. In fact, upcoming CLDR 38 resurrects such 
>>>>> patterns, so this fix is a precursor to support CLDR 38.
>>>>>
>>>>> Naoto
>>>>
>>



More information about the i18n-dev mailing list