ext/intl: introduce SpoofChecker::areBidiConfusable. - #13469
Conversation
b2f7b85 to
740f0df
Compare
| } | ||
| if (U_FAILURE(SPOOFCHECKER_ERROR_CODE(co))) { | ||
| php_error_docref(NULL, E_WARNING, "(%d) %s", SPOOFCHECKER_ERROR_CODE(co), u_errorName(SPOOFCHECKER_ERROR_CODE(co))); | ||
| RETURN_TRUE; |
There was a problem hiding this comment.
So we return true in case on failures? Is this behavior OK? Shouldn't we rather throw?
There was a problem hiding this comment.
was being consistent with the existing areConfusable method. I may adjust spoofchecker class behavior in a separate commit like I m doing with IntlTimeZone
There was a problem hiding this comment.
If the failures are related to programmatic errors, then it's surely better to throw. The one error case I can immediately see is however related to string length, which seems like a user-related one, but I guess it should be quite rare to exceed the limit. that's why I'm not really sure what to do. If we threw an exception, then the ugly reference param could also be avoided, which would be a nice advantage.
cc. @Girgias Do you have any opinion about this?
There was a problem hiding this comment.
Ehhhh not really, but shouldn't this go through the "normal" intl extension error handling mechanism so user can set if they want it to be silent/warning/error?
|
@devnexen Are you still interested in this? I'd be happy to supersede this because the code base has changed a lot since this PR was made and I think we need to rewrite this. |
740f0df to
c20ae74
Compare
Adding a new more refined spoofchecker method in addition of the existing Spoofchecker::areConfusable which takes in account the text direction left to right and right to left, along with the Spoofchecker::LTR and Spoofchecker::RTL constants. The self and typed references tests are extended accordingly, guarded at runtime rather than through a SKIPIF, so that the older ICU releases keep their coverage.
c20ae74 to
721167e
Compare
4 participants
Adding a new more refined spoofchecker method in addition of the existing SpoofChecker::areConfusable which takes in account the text direction
left to right and right to left.
Adding UBIDI_LTR, UBIDI_RTL, UBIDI_MIXED and UBIDI_NEUTRAL.