Thanks, Stephen.
On 11/14/17 3:22 PM, Stephen Colebourne wrote:
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.
OK.
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.
So even with the new suggested method,
formatter.withZone(locale).withLocalization(locale)
formatter.withLocalization(locale).withZone(locale)
would produce different formatters. Would it be OK, assuming along with
some proper documentation?
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.
Will document it.
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.
Would you suggest not interpreting extensions in this method? I am now
inclined to just interpret locale extensions in the new suggested method
for the java.time package.
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.
Sure.
I think there is a case for
CalendarDataUtility.retrieveFirstDayOfWeek() to return DayOfWeek, not
int, but perhaps that is a separate change.
Agree.
Naoto
Stephen
On 9 November 2017 at 23:34, Naoto Sato <naoto.s...@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
