Hi Naoto,
- The word "designated" is unnecessary and inconsistent with the rest of
the java.time API doc.
"designated locale" -> "locale"
I would have written: "Unicode extensions in the locale are ignored."
- localizedBy(Locale):
The first sentence should be more specific.
" Returns a copy of this formatter with localized values of the
locale, calendar, region and/or timezone, that supercede values in this
formatter."
to make it clearer that is is not a simple Locale substitution.
And update the @return to say the same
I would omit the example, to avoid the reader puzzling over what it
is that is changed. Or update the comment
to reinforce the point that the values are replaced when the
formatter is copied.
$.02, Roger
On 11/15/2017 4:18 PM, Naoto Sato wrote:
Here is the proposed spec changes:
http://cr.openjdk.java.net/~naoto/8191349/specdiff.00/overview-summary.html
Naoto
On 11/15/17 10:06 AM, Naoto Sato wrote:
On 11/14/17 4:44 PM, Stephen Colebourne wrote:
On 14 November 2017 at 23:58, Naoto Sato <naoto.s...@oracle.com> wrote:
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?
Thats why I suggested perhaps a different method name is needed, not
withXxx() to highlight the larger impact. eg. localizedBy(Locale)
OK. Will come up with a draft. Essentially what the new method does
is what withLocale() does, plus replacing fields specified with
Unicode extensions (calendar/timezone/region override)
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.
Fundamentally, the tags you are processing are a problem for the
design of java.time formatters. The existing API is structured around
a narrow meaning of Locale for text input/output within the formatting
API.
Changing the behaviour of DateTimeFormatterBuilder.toFormatter(...)
methods could have some odd effects for end-user code. Where they are
currently just expecting the locale to be set to control text
input/output, it would suddenly affect the calendar system and
time-zone, which could break code/compatibility in certain cases.
I've decided to retract changes in
DateTimeFormatter/DateTimeFormatterBuilder currently in the webrev,
with adding some text noting Unicode extensions are ignored in those
methods.
I think that its OK to use the unicode tags in places like
WeekFields.of() or Chronology.of(). But for formatting, the change in
meaning is too great. Adding a single method (name TBD) makes more
sense
Will keep the changes in WeekFields.of()/Chronology.of()
There is a case to add ZoneId.ofLocale(Locale) to match
Chronology.ofLocale(Locale). However, the expectation would be that it
figures out a suitable time-zone for the country/region as well as
considering the -u-tz- tag, and I don't think you've got that data
available at present (but it would make a good follow on change).
Yes, we don't yet have that mechanism to derive time zone based on
the region. I think that's out of this JEP's scope.
Naoto
