[10] RFR 8176841: Additional Unicode Language-Tag Extensions

[Stephen Colebourne]

I'd missed that this affects java.time.*

I believe that the changes to DateTimeFormatter are a mistake and will
cause confusion. Currently, each withXxx() method is independent -
they do not interact and operate on a single piece of state.

Currently, these two pieces of code have the same effect:

 formatter = formatter.withZone(zone).withLocale(locale);
 formatter = formatter.withLocale(locale).withZone(zone);

With the webrev, that is no longer true.

While I understand the desire of the change, I believe it would be a
serious mistake and a cause of end-user bugs.

The best I can suggest is adding a new method withLocalization(Locale)
or applyLocalization(Locale) or localized(Locale), which is documented
to replace multiple parts of the state in one go.


There are other oddities.
 DateTimeFormatterBuilder.getLocalizedDateTimePattern() takes in a
Chronology, thus the calendar system in the locale is ignored.
Probably correct, but not called out in the docs.

 DateTimeFormatterBuilder.toFormatter(Locale, ResolverStyle,
Chronology) has new logic to override the chronology. But this method
is used indirectly from ofLocalizedTime() and friends which require
the output to be ISO chronology. Thus the webrev would break the
specification of those methods.

TimeZoneNameUtility.convertLDMLShortID() is followed by ZoneId.of()
which assumes that a full set of time-zones is loaded. But, the ZoneId
initialization system allows the whole set of time-zones to be
replaced, making this logic suspect, or at the very least needing
extra consideration/testing.

I think there is a case for
CalendarDataUtility.retrieveFirstDayOfWeek() to return DayOfWeek, not
int, but perhaps that is a separate change.

Stephen


On 9 November 2017 at 23:34, Naoto Sato <naoto.sato at oracle.com> wrote:
> Kindly requesting reviews. I incorporated a fix to the following issue
> raised by the test team:
>
> https://bugs.openjdk.java.net/browse/JDK-8190918
>
> Here is the updated webrev:
>
> http://cr.openjdk.java.net/~naoto/8176841.8189134.8190918/webrev.04/
>
> And the webrev since the one below (to address 8190918):
>
> http://cr.openjdk.java.net/~naoto/8190918/
>
> Naoto
>
>
>
>
> On 11/2/17 2:42 PM, Naoto Sato wrote:
>>
>> Hello,
>>
>> Please review the proposed changes for the following issues:
>>
>> 8176841: Additional Unicode Language-Tag Extensions
>> 8189134: New system properties for the default Locale extensions
>>
>> The proposed changeset is located at:
>>
>> http://cr.openjdk.java.net/~naoto/8176841/webrev.03/
>>
>> This serves as the implementation of JEP 314.
>>
>> Naoto
>>
>>
>>
>