Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:56485
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain nebm.ist.utl.pt from 193.136.128.22 cause and error)
Content-Type: text/plain; charset=utf-8; format=flowed; delsp=yes
To: internals@lists.php.net
References: <4ECC9ABE.1010709@sugarcrm.com>
Date: Wed, 23 Nov 2011 09:03:29 -0000
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Organization: =?utf-8?Q?N=C3=BAcleo_de_Eng=2E_Biom=C3=A9di?=
 =?utf-8?Q?ca_do_IST?=
Message-ID: <op.v5d533h2idpuyk@cataphract.cata.lo.geleia.net>
In-Reply-To: <4ECC9ABE.1010709@sugarcrm.com>
User-Agent: Opera Mail/11.52 (Win32)
Subject: Re: [PHP-DEV] intl IDNA patch (was: Re: [PECL-DEV] libidn2 extension
 for php)
From: glopes@nebm.ist.utl.pt ("Gustavo Lopes")

On Wed, 23 Nov 2011 07:03:26 -0000, Stas Malyshev <smalyshev@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