[10] RFR: 8160199: Language's script should be reflected in user.script on Mac OS X

[Brent Christian]

Hi, Naoto

This looks quite good.

I will test a bit with some of the trickier locales.  BTW, the script 
will now also be reflected in Locale.getScript() and 
Locale.getDisplayScript().

Just a few comments:

---
src/java.base/macosx/native/libjava/java_props_macosx.h

Update copyright year

---
src/java.base/unix/native/libjava/locale_str.h

Update copyright year

214     "Hans", "Hans",
215     "Hant", "Hant",
216     "Latn", "Latn",
217     "Cyrl", "Cyrl",
218     "Arab", "Arab",

MacOS 10.12 ("Sierra") has a number of new languages not present in 
10.11, including a few new scripts.  I believe these should also be 
added here:

Deva
Ethi
Sund
Syrc
Tfng

---
src/java.base/macosx/native/libjava/java_props_macosx.c

Similar to the previous code, the call to:

79 
CFStringGetCString(CFLocaleGetIdentifier(CFLocaleCopyCurrent()),
80                                localeString, LOCALEIDLENGTH, 
CFStringGetSystemEncoding());

can be moved inside the if():

84             if (hyphenPos == NULL || // languageString contains 
ISO639 only, e.g., "en"
85                 languageString + langStrLen - hyphenPos == 5) {


FWIW, I believe this code path will only be taken for MacOS 10.11 and 
earlier.  Per TN2418[1] (and my testing thus far), 10.12 will always 
provide a region to 'languageString'.  10.11 and prior often return just 
a two-character language code.


134  * lang{_region}{@script}. e.g.,
135  * for "zh-Hans-US" into "zh_US at Hans"

This comment could fit on one line.


152             char tmp[4];

Maybe a more descriptive name ("tmpScript" ?)


173         assert((length == 3 || length == 8) &&
...

Idea: adding another set of ()s grouping:

   length 3 or 8 case
   length 4 or 9 case
   length == 5 case

would cause each case to highlight nicely in an IDE.  Just a thought.


A JDK 10 Reviewer will also need to take a look.

Thanks,
-Brent

1. https://developer.apple.com/library/content/technotes/tn2418/_index.html


On 6/28/17 10:51 AM, Naoto Sato wrote:
> Hello,
> 
> Please review the fix to the following issue:
> 
> https://bugs.openjdk.java.net/browse/JDK-8160199
> 
> The fix is located at:
> 
> http://cr.openjdk.java.net/~naoto/8160199/webrev.03/
> 
> The gist of the issue was that the script code has not been properly 
> treated in macOS specific code. Since it shares the same code with other 
> Unix, the locale format should follow POSIX, such as az-Cyrl-AZ needing 
> to be az_AZ at Cyrl.
> 
> Naoto