-
Notifications
You must be signed in to change notification settings - Fork 382
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-18056 skintone with facing right #4213
CLDR-18056 skintone with facing right #4213
Conversation
636c992
to
9bf86af
Compare
<annotation cp="🚶🏾♂➡" type="tts">homme qui marche vers la droite</annotation> | ||
<annotation cp="🚶🏿♂➡">homme | homme qui marche | marche | piéton</annotation> | ||
<annotation cp="🚶🏿♂➡" type="tts">homme qui marche vers la droite</annotation> | ||
<annotation cp="🚶➡">balade | droite | en route | flâner | homme | la | marche | marcher | nonchalant | personne | personne qui marche | piéton | promenade | se balader | vers</annotation> |
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.
Seems a little odd to have an article like "la" be a keyword by itself; perhaps we can refine this later.
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 agree. I was thinking about this, and we could really use a list of per-language 'stop words', like 'la' and 'en' above, that are ignored in matching and can thus be ignored in derivations and vetting. If you think that's a good idea I can file a ticket.
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 think that is a good idea (I was speculating about something like that), please do file a ticket. Such a list would probably also be useful elsewhere in CLDR/ICU.
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.
Filed a ticket and gave it to you for now.
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.
Annotations.java is much cleaner!
CLDR-18056
Adds skintones to the right-facing emoji. These are emoji that add joiner+right_facing_arrow to point to the right. It also fixes 2 other problems: the "facing right" was not added to annotations, and in the name it didn't have a colon before it like other modifiers.
Example 1, with skin tones
becomes
Example 2, without skin tones
becomes
Reviewer:
tools/cldr-code/src/main/java/org/unicode/cldr/util/Annotations.java
if (code.contains(EmojiConstants.JOINER_STRING)) {
got simpler, because we now just remove the arrow fromcode
, and add it torem
. That allows it to fall through properly toprivate Annotations getBasePlusRemainder(
, which handles the code.if (mod == BLACK_RIGHTWARDS_ARROW.codePointAt(0)) {
and just setting a flagtools/cldr-code/src/test/java/org/unicode/cldr/unittest/TestAnnotations.java
Restored TestUniqueness. That tests for exactly this kind of failure, and shouldn't have been removed unless a replacement was ready.
Added testCompleteness, that all emoji have English annotations.
Added testRightFacing, which has specific test cases for this problem.
This PR completes the ticket.
ALLOW_MANY_COMMITS=true