Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
Hi Stephen, The webrev has been updated accordingly. http://cr.openjdk.java.net/~sherman/8033662/webrev/ Thanks! -Sherman On 03/24/2014 08:59 AM, Stephen Colebourne wrote: I don't think think email was delivered to me. (And I think another from you also wasn't). The email is in the core-libs-dev archive though. I think the approach you changed it to is fine in principal although whether the tidy up should be in jdk8u is a different matter. In your changes I think that toParsed() should be renamed to toUnresolved(). And there still seem to be instance variables for effectiveZone and effectiveChrono (which I thought you were trying to remove). thanks Stephen On 24 March 2014 15:00, Xueming Shen xueming.s...@oracle.com mailto:xueming.s...@oracle.com wrote: Stephen, I commented on this one last week. I did not see the response, is it something worth considering? or you believe the original approach is the right way to go. Thanks! -Sherman Original Message Message-ID: 532b6f87.3000...@oracle.com mailto:532b6f87.3000...@oracle.com Date: Thu, 20 Mar 2014 15:45:27 -0700 From: Xueming Shen xueming.s...@oracle.com mailto:xueming.s...@oracle.com User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110414 Thunderbird/3.1.10 MIME-Version: 1.0 To: core-libs-dev@openjdk.java.net mailto:core-libs-dev@openjdk.java.net Subject:Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone() References: CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jevyv-d35+y...@mail.gmail.com mailto:CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jevyv-d35+y...@mail.gmail.com CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com mailto:CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com In-Reply-To: CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com mailto:CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi Stephen, Given the fact that the parser context is no longer public, the parsed from the formatter is either unresolved or resolved, just wonder if we really need those effective chrono and zone fields in Parsed. It appears perfect for me to simply keep these info inside the parser context and return the unresolved and resolved based on the formatter's request, for example http://cr.openjdk.java.net/~sherman/8033662/webrev2/ http://cr.openjdk.java.net/%7Esherman/8033662/webrev2/ -Sherman On 03/20/2014 11:24 AM, Stephen Colebourne wrote: Hi there, It would be great if I could get a review please. The patch is viewable in plain text at JIRA (for IP reasons): https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch The same patch is viewable in a nice format at GitHub https://gist.github.com/jodastephen/9505761 This really needs to make 8u20 IMO, so I need to get it into jdk9 first thanks Stephen On 12 March 2014 12:29, Stephen Colebournescolebou...@joda.org mailto:scolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed.Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
I'm happy with this updated patch. Stephen On 24 March 2014 18:24, Xueming Shen xueming.s...@oracle.com wrote: Hi Stephen, The webrev has been updated accordingly. http://cr.openjdk.java.net/~sherman/8033662/webrev/ Thanks! -Sherman On 03/24/2014 08:59 AM, Stephen Colebourne wrote: I don't think think email was delivered to me. (And I think another from you also wasn't). The email is in the core-libs-dev archive though. I think the approach you changed it to is fine in principal although whether the tidy up should be in jdk8u is a different matter. In your changes I think that toParsed() should be renamed to toUnresolved(). And there still seem to be instance variables for effectiveZone and effectiveChrono (which I thought you were trying to remove). thanks Stephen On 24 March 2014 15:00, Xueming Shen xueming.s...@oracle.com wrote: Stephen, I commented on this one last week. I did not see the response, is it something worth considering? or you believe the original approach is the right way to go. Thanks! -Sherman Original Message Message-ID: 532b6f87.3000...@oracle.com 532b6f87.3000...@oracle.com Date: Thu, 20 Mar 2014 15:45:27 -0700 From: Xueming Shen xueming.s...@oracle.comxueming.s...@oracle.com User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110414 Thunderbird/3.1.10 MIME-Version: 1.0 To: core-libs-dev@openjdk.java.net Subject: Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone() References: CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jevyv-d35+y...@mail.gmail.comCACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jevyv-d35+y...@mail.gmail.com CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.comCACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com In-Reply-To: CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.comCACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi Stephen, Given the fact that the parser context is no longer public, the parsed from the formatter is either unresolved or resolved, just wonder if we really need those effective chrono and zone fields in Parsed. It appears perfect for me to simply keep these info inside the parser context and return the unresolved and resolved based on the formatter's request, for example http://cr.openjdk.java.net/~sherman/8033662/webrev2/ -Sherman On 03/20/2014 11:24 AM, Stephen Colebourne wrote: Hi there, It would be great if I could get a review please. The patch is viewable in plain text at JIRA (for IP reasons): https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch The same patch is viewable in a nice format at GitHub https://gist.github.com/jodastephen/9505761 This really needs to make 8u20 IMO, so I need to get it into jdk9 first thanks Stephen On 12 March 2014 12:29, Stephen Colebournescolebou...@joda.org scolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
Hi there, It would be great if I could get a review please. The patch is viewable in plain text at JIRA (for IP reasons): https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch The same patch is viewable in a nice format at GitHub https://gist.github.com/jodastephen/9505761 This really needs to make 8u20 IMO, so I need to get it into jdk9 first thanks Stephen On 12 March 2014 12:29, Stephen Colebourne scolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
Hi Stephen, Given the fact that the parser context is no longer public, the parsed from the formatter is either unresolved or resolved, just wonder if we really need those effective chrono and zone fields in Parsed. It appears perfect for me to simply keep these info inside the parser context and return the unresolved and resolved based on the formatter's request, for example http://cr.openjdk.java.net/~sherman/8033662/webrev2/ -Sherman On 03/20/2014 11:24 AM, Stephen Colebourne wrote: Hi there, It would be great if I could get a review please. The patch is viewable in plain text at JIRA (for IP reasons): https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch The same patch is viewable in a nice format at GitHub https://gist.github.com/jodastephen/9505761 This really needs to make 8u20 IMO, so I need to get it into jdk9 first thanks Stephen On 12 March 2014 12:29, Stephen Colebournescolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
ping On 12 March 2014 12:29, Stephen Colebourne scolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
