RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

naoto.sato at oracle.com naoto.sato at oracle.com
Tue Aug 18 17:10:09 UTC 2020


Hi Roger,

Thank you for your comment. I added a brief comment in the issue on how 
the implementation behaves in the problem case.

Naoto

On 8/18/20 8:19 AM, Roger Riggs wrote:
> Hi Naoto,
> 
> I think the issue would benefit from a comment describing the solution.
> Its not clear how the code addresses the issue.
> 
> Thanks, Roger
> 
> 
> On 8/17/20 7: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 core-libs-dev mailing list