On Wed, 23 Nov 2011 07:03:26 -0000, Stas Malyshev <smalys...@sugarcrm.com>
wrote:
1. I'm not sure I understand why we need two options fields. We already
have one options field, won't that be enough? We can combine options and
space them in a way that old ones work fine with new ones, can't we?
And have default variant so one of them is always true (probably 2003).
Technically, yes, it is possible. But is it desirable? It would require
breaking the abstraction and looking at the actual values of the flags,
choosing one of the unused bits (possibly a high one) and hope it'll never
be used in future. Plus, in the current patch, the value of the variant
completely changes the return value (from string to array) so if it's more
appropriate to single it out.
2. I think optional by-ref parameter to receive IDNA-specific error
codes is right. Generic intl error reporting is about reporting, well,
generic errors and is not meant to have granularity enough to store
whole UIDNAInfo stuff.
That's what they've must felt lately too, because the IDNA2003
implementation uses the generic intl error reporting.
I don't really care or way or the other about pass-by-ref vs mixed return,
but I don't think your position on that issue is clear here, as you only
affirm that trying to force generic intl error reporting is not an option
(which I've also argued). If there are no objections, I'll advance with
mixed return, as Pierre has announced.
3. I'm not sure I understand the code in php_intl_bad_args() - it
composes error message into the buff but then only sets it if msg !=
NULL. It doesn't check msg before using it in sprintf. Looks like
something's missing there: I don't see how msg can be NULL there at all
but if we check it, let's check it right :) Another place has the same
code.
Ah yes, nice catch. Not that it hurts because the functions are never
called with NULL and they're static, but that part is nonsensical.
4. Also we'd want some tests with that ;)
Sure. I will also test both against ICU 4.2 and 4.8.
--
Gustavo Lopes
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
