Please Review fix for reduced value parser 8024076

roger riggs roger.riggs at oracle.com
Tue Oct 1 18:51:08 UTC 2013


Hi Stephen,

The proposed approach makes sense to me, shall I take the patch as is or
wait to integrate until for the mentioned update of effective chrono?

Thanks, Roger

On 9/22/2013 10:27 AM, Stephen Colebourne wrote:
> The patch only changes the text of one of the two appendValueReduced
> methods. The patch does not handle week based years or provide for
> users to add their own year fields. It also does not handle
> formatting.
>
> After much thinking, I think the right solution is to add a new
> appendValueReduced method where "int baseValue" is replaced by
> "ChronoLocalDate baseDate". The new method would be used if you want
> year-like fields in multiple chronologies to work. The appendPattern
> method would be changed to use the new date variant for y/u/W
>
> The first of the two existing appendValueReduced methods can be
> removed as a simplification.
>
> Patch here:
> https://gist.github.com/jodastephen/6660394
>
> Note that this patch still has a bug, as the effective chrono is not
> determined fully until the end of the parsing phase. However, that bug
> fix requires a bit of an internal redesign and since it does not
> affect the API it can be delayed,
>
> Stephen
>
>
>
>
> On 21 September 2013 20:15, roger riggs <roger.riggs at oracle.com> wrote:
>> Hi,
>>
>> The java.time reduced value parser does work as expected (issue 8024076)
>> for chronologies other than ISO.
>> The base value is assumed to be chronology independent but is not
>> converted to the requested Chronology before it is used.
>>
>> Please review:
>>
>>      http://cr.openjdk.java.net/~rriggs/webrev-two-digit-8024076/
>>
>> Thanks, Roger
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8024076
>>




More information about the core-libs-dev mailing list