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

Joe Wang huizhe.wang at oracle.com
Tue Aug 18 06:52:31 UTC 2020


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