RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()

[Xueming Shen]

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 <mailto: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> <mailto:532B6F87.3000007 at oracle.com>
>     Date: 	Thu, 20 Mar 2014 15:45:27 -0700
>     From: 	Xueming Shen <xueming.shen at oracle.com> <mailto: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 <mailto: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> <mailto:CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jEvYv-D35+yoeQ at mail.gmail.com> <CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at mail.gmail.com> <mailto:CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at mail.gmail.com>
>     In-Reply-To: 	<CACzrW9BcmmesdKQBrtgrAc=12HN3uowJVwZxSKMfCKsAJeanfg at mail.gmail.com> <mailto: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/  <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 Colebourne<scolebourne at joda.org> <mailto: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
>
>
>
>