-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[SE-0211] Emoji properties require ICU 57 or later #19209
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
Conversation
When I last brought this up, @milseman in this comment felt that the properties shouldn't be gated with availability annotations in this comment. Reading over it again, I probably disagree with the first sentence "The properties are available"; before the version of ICU that implemented the corresponding version of the standard, the properties aren't available, and there's no way to use them on an older OS that produces the right result. That's distinct from the grapheme cluster behavior alluded to in But I'd like to hear if Michael has any other concerns about this. Sadly for Linux, there's no way to gate the properties based on version there, but that's ok, because the long-term solution is to bundle ICU with stdlib to ensure that it always contains what we need. |
Right, if the API isn't even available, then we need to disable it on that platform version. IIRC, I was referring to properties that were available but gave a different answer than what we were testing for. |
I regret not being clearer then in my original comment on the other PR. 🙂 In that case, I agree completely—let's do this, because the properties don't exist at all on the OS versions before those listed above. |
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.
Minor test change requested, LGTM
ef1a6a4
to
cec9faf
Compare
@swift-ci please test and merge |
|
@swift-ci Please test |
Build failed |
Build failed |
This is ready to merge (all checks have passed). |
Availability of emoji properties should be platform versions which have a
/usr/lib/libicucore.dylib
derived from ICU 57 or later.These platform versions are based on the
unicode/uchar.h
andunicode/uvernum.h
headers of:(I didn't check as far back as macOS 10.9 or iOS 7).
This pull request is a follow-up to: