RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone() (original) (raw)

Stephen Colebourne scolebourne at joda.org
Mon Mar 24 18:55:44 UTC 2014

I'm happy with this updated patch. Stephen

On 24 March 2014 18:24, Xueming Shen <xueming.shen at 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.shen at 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.3000007 at oracle.com> <532B6F87.3000007 at oracle.com> Date: Thu, 20 Mar 2014 15:45:27 -0700 From: Xueming Shen <xueming.shen at oracle.com><xueming.shen at 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 at openjdk.java.net Subject: Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone() References: <CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jEvYv-D35+yoeQ at mail.gmail.com><CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jEvYv-D35+yoeQ at mail.gmail.com> <CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at mail.gmail.com><CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at mail.gmail.com> In-Reply-To: <CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at mail.gmail.com><CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at 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 Colebourne<scolebourne at joda.org> <scolebourne at 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

More information about the core-libs-dev mailing list