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...@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
--
Thanks and Regards,
Nadeesh TV
