RFR:JDK-8066806:java.time.format.DateTimeFormatter cannot parse an offset with single digit hour

[Stephen Colebourne]

The test test_strict_offset_adjacentValidPattern_parse should also
check that the hour of day was parsed to 12.

I don't think there are any tests of the lenient behaviour?

thanks
Stephen

On 10 June 2016 at 12:17, nadeesh tv <nadeesh.tv at oracle.com> wrote:
> Hi,
>
> Please see the updated webrev
> http://cr.openjdk.java.net/~ntv/8066806/webrev.08/
>
> Thanks and Regards,
> Nadeesh
>
> On 6/9/2016 4:29 PM, nadeesh tv wrote:
>>
>> Hi Stephen,
>>
>> On 6/9/2016 4:19 PM, Stephen Colebourne wrote:
>>>
>>> "absHours / 10 > 0" would be simpler as "absHours >= 10"
>>>
>>> Around line 3595 we have
>>>   boolean paddedHour = isPaddedHour();
>>> but 6 lines down isPaddedHour() is used, not the local variable
>>>
>>> There is an extra space in the message here:
>>>   new DateTimeException(" Value out of Range
>>> also, I'd use "range", not "Range".
>>
>> Thanks.
>>>
>>>
>>> The maximum for ZoneOffset is actually 18:00:00 not 23:59:59, however
>>> it is not worth validating that here. The base validation for 23/59/59
>>> that has been added is just fine, values between 18 and 23 will be
>>> rejected later in the processs when attempting to create an instance
>>> of ZoneOffset.
>>>
>>> I don't see any tests for
>>>
>>> new DateTimeFormatterBuilder().appendOffset(pattern,
>>> "Z").appendValue(ChronoField.NANO_OF_DAY).toFormatter().parse(offset +
>>> "12345")
>>>
>>> which should work for most of the patterns.
>>
>> I intentionally avoided it. I will create a positive test cases for
>> working patterns and negative test cases for rest.
>>
>> Regards,
>> Nadeesh
>>>
>>>
>>> thanks
>>> Stephen
>>>
>>> On 9 June 2016 at 10:27, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>>>>
>>>> Hi Roger,
>>>> Thanks for the comments.
>>>>
>>>> Please see the updated webrev
>>>> http://cr.openjdk.java.net/~ntv/8066806/webrev.07/
>>>> Thanks and regards,
>>>> Nadeesh
>>>>
>>>> On 6/9/2016 2:36 AM, Roger Riggs wrote:
>>>>
>>>> HI Nadeesh,
>>>>
>>>> Looking better
>>>>
>>>> DateTimeFormatterBuilder:
>>>>
>>>>   - line 3678: If array[1] == 24, offsetSeconds will be greater that
>>>> seconds
>>>> in a day;  that's not right.
>>>>     I don't think hour=24 is valid.  (and there would be test case(s)
>>>> for
>>>> it.)
>>>>
>>>> There should be test cases for offsets over the limit of hours, minutes,
>>>> and
>>>> seconds: 24:60:60
>>>>
>>>> Thanks, Roger
>>>>
>>>>
>>>>
>>>> On 6/8/2016 2:59 AM, nadeesh tv wrote:
>>>>
>>>> Hi,
>>>>
>>>> Please see the updated webrev
>>>> http://cr.openjdk.java.net/~ntv/8066806/webrev.06/
>>>>
>>>> I reused code provided by Stephen and handled the edge cases accordingly
>>>>
>>>> Thanks and Regards,
>>>> Nadeesh
>>>>
>>>> On 5/31/2016 7:15 PM, Stephen Colebourne wrote:
>>>>
>>>> Where the new patterns are described in Javadoc, there is no
>>>> discussion of the difference between "H" and "HH".
>>>>
>>>> Add after </ul>
>>>>
>>>> "Patterns containing "HH" will format and parse a two digit hour,
>>>> zero-padded if necessary. Patterns containing "H" will format with no
>>>> zero-padding, and parse either one or two digits."
>>>>
>>>> "with colo" should be "with colon"
>>>>
>>>> As for the main code, I've had a go at a rewrite:
>>>> https://gist.github.com/jodastephen/68857dd344e33bd6c0b3b4d24279d2e4
>>>>
>>>> It is completely untested, and surely has mistakes, however as a
>>>> design it seems reasonable.
>>>>
>>>> I agree that the tests need to cover these cases:
>>>>
>>>> - offset at end of line
>>>> - offset followed by letters
>>>> - offset followed by numbers
>>>>
>>>> Stephen
>>>>
>>>>
>>>> On 26 May 2016 at 08:49, nadeesh tv <nadeesh.tv at oracle.com> wrote:
>>>>
>>>> Hi all,
>>>>
>>>> Please review
>>>>
>>>> BugId : https://bugs.openjdk.java.net/browse/JDK-8066806
>>>>
>>>> Issue: java.time.format.DateTimeFormatter cannot parse an offset with
>>>> single
>>>> digit hour
>>>>
>>>> webrev:  http://cr.openjdk.java.net/~ntv/8066806/webrev.03/
>>>>
>>>> Solution: Added the suggested patterns but the parsing logic became too
>>>> complex.
>>>>    Appreciate any suggestion to make the  parsing less complicated
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Nadeesh TV
>>>>
>>>>
>>>>
>>>>
>>>> --
>>>> Thanks and Regards,
>>>> Nadeesh TV
>>>>
>>
>
> --
> Thanks and Regards,
> Nadeesh TV
>