RFR:JDK-8145633-Adjacent value parsing not supported for Localized Patterns

Roger Riggs roger.riggs at oracle.com
Wed Dec 21 14:16:49 UTC 2016


Hi Nadeesh,

Looks fine.

It would be more direct to add that "The inherited field 
NumberPrinterParser.field is unused."

No need for another review cycle.

Thanks, Roger


On 12/21/16 8:24 AM, nadeesh tv wrote:
> Hi Roger & Stephen,
>
> Please see the updated webrev 
> http://cr.openjdk.java.net/~ntv/8145633/webrev.13/
>
>
> On 12/21/2016 3:11 AM, Roger Riggs wrote:
>> Hi Nadeesh,
>>
>> On 12/20/2016 2:34 PM, nadeesh tv wrote:
>>>
>>> Hi Roger & Stephen ,
>>> Thanks for the comments.
>>>
>>> Please see the updated webrev 
>>> http://cr.openjdk.java.net/~ntv/8145633/webrev.12/
>>>
>>> Changes included :
>>>
>>> 1. Doc changes and cosmetic changes suggested by Roger (except the 
>>> note about delay..)
>> The comments at 4776-4779 are fine.
>> The issue came from passing null at line 4809 to the 
>> NumberPrinterParser constructor
>> that expects the field argument to be non-null, and wanting some 
>> explanation of that disconnect.
>
> Changes included:  Added the following clarification.
> 4775      * Prints or parses a localized pattern from a localized field.
> 4776      * The specific formatter and parameters is not selected until the
> 4777      * the field is to be printed or parsed.
> 4778      * The locale is needed to select the proper WeekFields from which
> 4779      * the field for day-of-week, week-of-month, or week-of-year is selected.
> *4780 * Since Locale is available only during parsing or formatting, 
> the WeekBasedField 4781 * will be null during construction.*
> Is it OK?
>
> Thanks and Regards,
> Nadeesh
>>
>> Other than that, I'm fine with the changes.
>>
>> Roger
>>
>>>
>>> 2.  Changed the  behavior of 'e' to parse only 1 digit as suggested 
>>> by Stephen.  Changed the existing test cases for this.
>>>
>>> Thanks and Regards,
>>> Nadeesh
>>>
>>> On 12/20/2016 10:48 PM, Stephen Colebourne wrote:
>>>> In the test provider_adjacentValuePatterns2(), please add
>>>>
>>>> {"YYYYwwe", Y, w, c, "20161201", 2016, 12, 1},
>>>>
>>>> This should succeed, because a single number is all that is needed to
>>>> parse day-of-week. (So, it will need to be removed from the invalid
>>>> patterns test).
>>>>
>>>> Line 1869 will need to change to "count, count, count" to make the 
>>>> tests pass.
>>>>
>>>> Otehrwise, looks fine, thanks.
>>>> Stephen
>>>>
>>>>
>>>> On 20 December 2016 at 09:55, nadeesh tv <nadeesh.tv at oracle.com> 
>>>> wrote:
>>>>> Hi all,
>>>>>
>>>>> BugId: https://bugs.openjdk.java.net/browse/JDK-8145633
>>>>>
>>>>> Issue:  Support adjacent value parsing for  Localized Patterns
>>>>>
>>>>> Webrev: http://cr.openjdk.java.net/~ntv/8145633/webrev.10/
>>>>>
>>>>> Pattern 'c' and 'W'  were previously allowed to have 'zero 
>>>>> padding' which
>>>>> was not explicitly mentioned in CLDR
>>>>> (http://unicode.org/reports/tr35/tr35-dates.html).
>>>>> To allow 'c' and 'W' to take part in adjacent value parsing ( at 
>>>>> the same
>>>>> time, 2 digits are  not required for these patterns), restricted 
>>>>> the max
>>>>> width of these patterns to 1.
>>>>>
>>>>> Special thanks to Stephen for the help.
>>>>>
>>>>> -- 
>>>>> Thanks and Regards,
>>>>> Nadeesh TV
>>>>>
>>>
>>
>
> -- 
> Thanks and Regards,
> Nadeesh TV
>



More information about the core-libs-dev mailing list