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

[nadeesh tv]

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