Looks good to me.
Masayoshi
On 11/8/2012 4:23 AM, Naoto Sato wrote:
Thanks. I incorporated your comments and generated a new webrev:
http://cr.openjdk.java.net/~naoto/8001205.8001562/webrev.01/
Naoto
On 11/7/12 1:24 AM, Masayoshi Okutsu wrote:
Hi,
Here are my comments.
- LocaleServiceProviderPool.java:
- * provider, including the JRE.
+ * provider, including the JRE's FormatData locales.
*
* @return an array of the available locales
*/
public Locale[] getAvailableLocales() {
- Set<Locale> locList = getAvailableLocaleList();
+ Set<Locale> locList = new HashSet<>();
+ locList.addAll(getAvailableLocaleSet());
+ // Make sure it all contains JRE's FormatData locales for
compatibility.
It's not accurate to say FormatData locales. The FormatData locales list
is no longer a superset.
- <> should be used where it's applicable.
- test/java/util/Locale/Bug8001562.java: line 37 is too long...
Otherwise, the fixes look good to me.
Masayoshi
On 11/7/2012 7:17 AM, Naoto Sato wrote:
Eventually, I merged the fix for 8001562 with 8001205, and here is the
combined webrev:
http://cr.openjdk.java.net/~naoto/8001205.8001562/webrev.00/
This version includes the following:
- Previous fix for 8001205
- Fix for 8001562, where available locales for JRE's FormatData
locales are now included for getAvailableLocales() return values for
all locale sensitive services (e.g. DateFormat)
- Removed the code for getting rid of Locale.ROOT from the available
locales. Returning it will not affect the compatibility with JDK7
(it's a superset).
- Method name change (getAvailableLocaleList() ->
getAvailableLocaleSet()).
Please review.
Naoto
On 11/2/12 12:02 PM, Naoto Sato wrote:
Will fix 8001562 too, with a separate changeset.
Naoto
On 11/2/12 1:08 AM, Masayoshi Okutsu wrote:
Do you plan to fix this one with 8001562? (If you fix only this
one, it
may be seen as a regression.)
Masayoshi
On 10/31/2012 11:55 AM, Naoto Sato wrote:
With this fix, no. But another bug was filed regarding the locale
sensitive service APIs' (e.g., Collator) getAvailableLocales()
return
value (8001562). I will fix it to keep the returned locale array
compatible with JDK7.
Naoto
On 10/30/12 7:43 PM, Masayoshi Okutsu wrote:
Question. Does the full set of available locales of JRE still
become
available with this fix? For example, when
java.locale.providers=SPI,
all JRE locales are included in the getAvailableLocales (API)? The
fallback adapter is an interesting idea, but it's a bit
confusing...
Thanks,
Masayoshi
On 10/31/2012 6:44 AM, Naoto Sato wrote:
Hello,
Please review the fix for the following bug:
http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=8001205
The fix is simply to supply the fallback locale provider
adapter for
the root locale, when there is no explicit "JRE" adapter is
specified
in the locale provider preference list.
The proposed fix is located at:
http://cr.openjdk.java.net/~naoto/8001205/webrev.00/
Naoto
