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

[naoto.sato at oracle.com]

Hi Roger,

Thank you for your comments. I've addressed your suggestions and created 
a new webrev below:

https://cr.openjdk.java.net/~naoto/8251499/webrev.03/

Naoto

On 8/19/20 8:33 AM, Roger Riggs wrote:
> Hi Naoto,
> 
> CompactNumberFormat.java:
> 
> 269: The field "placeHolders" should be named consistently with the 
> other holders of Patterns.
>    -> placeHolderPatterns
> 
> 1632: missing space before ":"
> 
> 2390:  I'm not sure why the stream processing is preferable to a direct 
> forEach(...).
> 
> TestCompactPatternsValidity:
> For the Plurals cases it would clearer to create a new Data provider and 
> corresponding tests
> with the extra arg.
> 
> Thanks, Roger
> 
> 
> 
> On 8/18/20 5:13 PM, Joe Wang wrote:
>> 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
>>>>>>
>>>>
>>
>