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

[nadeesh tv]

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