RFR:JDK-8079628:java.time: DateTimeFormatter containing "DD" fails on 3-digit day-of-year value

[nadeesh tv]

Hi Roger,

Ok. Thanks.

Regards,
Nadeesh

On 5/4/2016 7:58 PM, Roger Riggs wrote:
> Hi Nadeesh,
>
> java/time/format/DateTimeFormatterBuilder.java: 1836-1839
>
> Correct as is, but could collapse both count ==2 and count ==3 into a 
> single appendValue call:
>
> appendValue(field, count, 3, SignStyle.NOT_NEGATIVE); Reviewed.
>
> Roger
>
>
> On 5/4/2016 3:15 AM, nadeesh tv wrote:
>> Hi,
>> Updated the webev http://cr.openjdk.java.net/~ntv/8079628/webrev.04/
>> Regards,
>> Nadeesh
>> On 5/3/2016 8:45 PM, Stephen Colebourne wrote:
>>> Letters "Q", "q", "M", "L", "d", "D", "F", "h", "H", "k", "K", "m",
>>> "s" and no doubt others use NORMAL via
>>>
>>>   appendValue(field);
>>>
>>> Changing these to use NOT_NEGATIVE would be a big change and doesn't
>>> seem justified.
>>>
>>> For "D", "DD" and "DDD" these seem to be the best balance (as
>>> discussed up-thread):
>>>
>>>   *    D       1      appendValue(ChronoField.DAY_OF_YEAR)
>>>   *    DD      2      appendValue(ChronoField.DAY_OF_YEAR, 2, 3,
>>> SignStyle.NOT_NEGATIVE)
>>>   *    DDD     3      appendValue(ChronoField.DAY_OF_YEAR, 3)
>>>
>>> ie. we use NOT_NEGATIVE when we are making changes to a field and that
>>> is the best option, but otherwise we stick with NORMAL for single
>>> letter patterns like "D".
>>>
>>> Stephen
>>>
>>>
>>> On 3 May 2016 at 15:22, Roger Riggs <Roger.Riggs at oracle.com> wrote:
>>>> Hi Nadeesh,
>>>>
>>>>
>>>> On 5/3/2016 3:24 AM, nadeesh tv wrote:
>>>>
>>>> Hi Roger,
>>>> Please see the answers inline
>>>>
>>>> On 5/3/2016 2:43 AM, Roger Riggs wrote:
>>>>
>>>> Hi Nadeesh,
>>>>
>>>> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
>>>>
>>>>
>>>> line 1835: the appendValue(field) method has a sign-style of 
>>>> NORMAL; should
>>>> that be NOT_NEGATIVE?
>>>>
>>>> It's 'NOT_NEGATIVE' only.  May be a browser caching issue. Can you  
>>>> please
>>>> check it once again.
>>>>
>>>> My question was about the existing case for "D" that uses
>>>> appendValue(field).
>>>> That is a shorthand for:
>>>>      appendValue(new NumberPrinterParser(field, 1, 19, 
>>>> SignStyle.NORMAL));
>>>>
>>>> test/java/time/tck/java/time/format/TCKDateTimeFormatterBuilder.java:
>>>>
>>>> What is being tested in test_dayOfYearFieldPattern?
>>>> It does not check that the value parsed matches the input.
>>>>
>>>> These test cases added initially. Now it's become almost redundant 
>>>> due to
>>>> 'dayOfYearFieldValues' . I could remove it.
>>>>
>>>> If they are redundant, they should be removed.
>>>>
>>>>
>>>> test/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java
>>>>
>>>> 686, 688: should these cases show with sign-style NOT_NEGATIVE also?
>>>>
>>>> Suspecting browser caching here also.
>>>>
>>>> I'm referring to the test case for "D", which seems like it should 
>>>> show up
>>>> as NOT_NEGATIVE.
>>>>
>>>>
>>>> Updated the webrev  by removing some unnecessary spaces
>>>> http://cr.openjdk.java.net/~ntv/8079628/webrev.03/
>>>>
>>>>
>>>> Thanks, Roger
>>>>
>>
>

-- 
Thanks and Regards,
Nadeesh TV