-
Notifications
You must be signed in to change notification settings - Fork 388
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CLDR-16403 Add v44 PersonName features #3304
CLDR-16403 Add v44 PersonName features #3304
Conversation
Ping? |
Looking at it now, was backed up behind some other also-urgent things |
@@ -135,6 +135,13 @@ The following features are currently out of scope for Person Names formating: | |||
|
|||
A draft API for formatting personal names is included in ICU4J 73. (“Draft” means that the full functionality is present, but the API might be refined before it is stabilized.) The implementation can be found at [PersonNameFormatter.java](https://github.com/unicode-org/icu/blob/main/icu4j/main/classes/core/src/com/ibm/icu/text/PersonNameFormatter.java) and [SimplePersonName.java](https://github.com/unicode-org/icu/blob/main/icu4j/main/classes/core/src/com/ibm/icu/text/SimplePersonName.java). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will make a separate PR to update this line.
I am going to go ahead and merge this. Any follow-on fixes can go into a separate PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Belated review. Generally looks great, but there were a couple things in here that surprised me-- I've called those out below.
|
||
1. forceGivenFirst — no matter what the values are in nameOrderLocales or in the NameObject, display the name as givenFirst. | ||
2. forceSurnameFirst — no matter what the values are in nameOrderLocales or in the NameObject, display the name as surnameFirst. | ||
3. forceNativeOrdering — no matter what the values are in nameOrderLocales or in the NameObject, display the name with the same ordering as the native locale. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that it matters, but I didn't implement this in ICU. I think that was partly because what we had at the time wasn't clear on just what this was supposed to do, but also because what I thought it was supposed to do was going to require a lot of extra work in the code that wouldn't normally be worth it. Is this really worth doing? If so, you might want to file an ICU ticket.
1. forceGivenFirst — no matter what the values are in nameOrderLocales or in the NameObject, display the name as givenFirst. | ||
2. forceSurnameFirst — no matter what the values are in nameOrderLocales or in the NameObject, display the name as surnameFirst. | ||
3. forceNativeOrdering — no matter what the values are in nameOrderLocales or in the NameObject, display the name with the same ordering as the native locale. | ||
4. surnameFirstAllCaps — display the surname and surname2 fields in all caps **if** not using native order. Thus where the foreign name ordering is surnameFirst, the name {given=Shinzo, surname=Abe} would display as “ABE Shinzo”. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think I implemented this this way-- I think my ICU code always capitalizes the surname, regardless of field order. This is the first I've heard that we wanted to restrict that behavior to just names that are displayed in the opposite of their normal order. Again, (without looking at the code right now) this seems like a significant amount of extra work at run time to get and check the order-- is it really worth it? If so, please file an ICU ticket.
CLDR-16403
There is a lot of overlap between this ticket and https://unicode-org.atlassian.net/browse/CLDR-17124. I went ahead and included the extra text for nativeSpaceReplacement (replacing some old text), so I think we can mark https://unicode-org.atlassian.net/browse/CLDR-17124 as a duplicate.
ALLOW_MANY_COMMITS=true