"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".
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.
thanks
Stephen
On 9 June 2016 at 10:27, nadeesh tv <nadeesh...@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...@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
>
