ext/intl: introduce SpoofChecker::areBidiConfusable. #13469
Conversation
devnexen
force-pushed
the
ext_intl_spoof_arebidirectionalconfusable
branch
from
768115a to
b2f7b85
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. Adding UBIDI_LTR, UBIDI_RTL, UBIDI_MIXED and UBIDI_NEUTRAL.
devnexen
force-pushed
the
ext_intl_spoof_arebidirectionalconfusable
branch
from
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?
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.