Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-03-04 Thread Stephen Colebourne
In TCKDateTimeFormatterBuilder there is a commented out line in one of the new tests. Should be removed. No need for another review for that - happy otherwise. thanks for the work! Stephen On 3 March 2016 at 18:37, nadeesh tv wrote: > Hi, > > Stephen, Roger Thanks for the

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-03-03 Thread Roger Riggs
Hi Nadeesh, Looks good. Thanks, Roger On 3/3/2016 1:37 PM, nadeesh tv wrote: Hi, Stephen, Roger Thanks for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.04/ Regards, Nadeesh On 3/1/2016 12:29 AM, Stephen Colebourne wrote: I'm happy to go

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-03-03 Thread nadeesh tv
Hi, Stephen, Roger Thanks for the comments. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.04/ Regards, Nadeesh On 3/1/2016 12:29 AM, Stephen Colebourne wrote: I'm happy to go back to the spec I proposed before. That spec would determine colons dynamically

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-29 Thread Stephen Colebourne
I'm happy to go back to the spec I proposed before. That spec would determine colons dynamically only for pattern HH. Otherwise, it would use the presence/absence of a colon in the pattern as the signal. That would deal with the ISO-8601 problem and resolve the original issue (as

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-29 Thread Roger Riggs
Hi Stephen, As a fix for the original issue[1], not correctly parsing a ISO defined offset, the use of lenient was a convenient implementation technique (hack). But with the expanded definition of lenient, it will allow many forms of the offset that are not allowed by the ISO specification

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
It is important to also consider the case where the user wants to format using HH:MM but parse seconds if they are provided. As I said above, this is no different to SignStyle, where the user requests something specific on format, but accepts anything on input. The pattern is still used for

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Roger Riggs
Hi Stephen, Even in lenient mode the parser needs to stick to the fields provided in the pattern. If the caller intends to parse seconds, the pattern should include seconds. Otherwise the caller has not been able to specify their intent. That's consistent with lenient mode used in the other

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
On 26 February 2016 at 15:00, Roger Riggs wrote: > Hi Stephen, > > It does not seem natural to me with a pattern of HHMM for it to parse more > than 4 digits. > I can see lenient modifying the behavior as it it were HHmm, but there is no > indication in the pattern > that

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Roger Riggs
Hi Stephen, It does not seem natural to me with a pattern of HHMM for it to parse more than 4 digits. I can see lenient modifying the behavior as it it were HHmm, but there is no indication in the pattern that seconds would be considered. How it would it be implied from the spec? In the

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
Lenient can be however lenient we define it to be. Allowing minutes and seconds to be parsed when not specified in the pattern is the key part of the change. Whether the parser copes with both colons and no-colons is the choice at hand here. It seems to me that since the parser can easily handle

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Roger Riggs
HI Stephen, How lenient is lenient supposed to be? Looking at the offset test cases, it seems to allow minutes and seconds digits to be parsed even if the pattern did not include them. + @DataProvider(name="lenientOffsetParseData") + Object[][] data_lenient_offset_parse() { + return new

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-26 Thread Stephen Colebourne
I don't think this is quite right. if ((length > position + 3) && (text.charAt(position + 3) == ':')) { parseType = 10; } This code will *always* select type 10 (colons) if a colon is found at position+3. Whereas the spec now says that it should only do this if the pattern is "HH". For other

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-25 Thread nadeesh tv
Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.02/ Thanks and Regards, Nadeesh On 2/23/2016 5:17 PM, Stephen Colebourne wrote: Thanks for the changes. In `DateTimeFormatter`, the code should be .parseLenient() .appendOffsetId() .parseStrict() and the

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-23 Thread Stephen Colebourne
Thanks for the changes. In `DateTimeFormatter`, the code should be .parseLenient() .appendOffsetId() .parseStrict() and the same in the other case. This ensures that existing callers who then embed the formatter in another formatter (like the ZONED_DATE_TIME constant) are unaffected. The

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-22 Thread Roger Riggs
Hi Nadeesh, Sorry for the delay. DateTimeFrmatterBuilder.java: 3387: avoid testing isStrict twice. - refactor the if's so there is an outer if for context.isStrict == false and inner if's (or tertiary ?: assignment) for the replacement parseType. if (context.isStrict() == false) {

Re: RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-22 Thread nadeesh tv
Gentle Reminder On 2/12/2016 1:52 AM, nadeesh tv wrote: Hi all, Please review a fix for Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051 webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/ -- Thanks and Regards, Nadeesh TV

RFR:JDK-8032051:"ZonedDateTime" class "parse" method fails with short time zone offset ("+01")

2016-02-11 Thread nadeesh tv
Hi all, Please review a fix for Bug Id https://bugs.openjdk.java.net/browse/JDK-8032051 webrev http://cr.openjdk.java.net/~ntv/8032051/webrev.01/ -- Thanks and Regards, Nadeesh TV