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
Eliminate ICU for OSX #6949
Eliminate ICU for OSX #6949
Conversation
339aa77
to
ab9d28c
Compare
CFRelease(cf1); | ||
CFRelease(cf2); | ||
|
||
return (int)res + 2; |
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.
given the only usage of this is to just do -2 on the result again, why not make the actual return value consistent with what people expect from a sorting function (-1, 0, 1)? Seems to me like the supported
check should be done elsewhere
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.
Oh, OTTDStringCompare does the same. bleh.
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.
The supported check is platform-specific, the call site is not. As such, we need a value for failed as well.
By default, the native API will be used instead of ICU, but if ICU is forced in using configure, it will take precedence.
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.
Tested on macOS 10.13.6. Can't see any issues.
- tested font display, list sorting and text caret behaviour
- tested roman (original sprite font), left-to-right
Also tested
- Arabic (Egypt) right-to-left with a system font
- Korean, left-to-right with a system font
No obvious issues, but I don't know what's expected behaviour there.
This PR provides native OSX implementations for string sorting, caret handling, and text layout. While it is still possible to use ICU for these on OSX, the default build will not depend on it anymore.