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

[nadeesh tv]

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