Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:6379
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
In-Reply-To: <200312121728.44466.ilia@prohost.org>
References: <25BBBBC2-2CD2-11D8-8FCC-000A95CE0C62@at.wakwak.com> <200312121551.25399.ilia@prohost.org> <C7B74660-2CE8-11D8-9A2A-000A95CE0C62@at.wakwak.com> <200312121728.44466.ilia@prohost.org>
Mime-Version: 1.0 (Apple Message framework v606)
Content-Type: text/plain; charset=US-ASCII; format=flowed
Message-ID: <F9C15AC8-2CF3-11D8-9A2A-000A95CE0C62@at.wakwak.com>
Content-Transfer-Encoding: 7bit
Cc: PHP Internals <internals@lists.php.net>
Date: Sat, 13 Dec 2003 07:38:58 +0900
To: ilia@prohost.org
Subject: Re: [PHP-DEV] Re: Regarding the latest patch on fgetcsv() (stable branch)
From: moriyoshi@at.wakwak.com (Moriyoshi Koizumi)


On 2003/12/13, at 7:28, Ilia Alshanetsky wrote:

> On December 12, 2003 04:18 pm, Moriyoshi Koizumi wrote:
>> I disagree, because of the following reasons:
>>
>> 1) Not a few people *actually* use fgetcsv() commonly
>>     with multibyte characters indeed. Regarding this,
>>     applications made by those who don't use
>>     such characters don't (and won't) use multibyte specific
>>     functions and that's the problem. This greatly prevents
>>     them from being portable.
>
> People have lived without multibyte support in fgetcsv() for many 
> years now,
> and I did not see a single request on bugs.php.net for fgetcsv() 
> multi-byte
> support. So, while this is certainly useful functionality I do not 
> believe it
> is as widely needed as you say it is. We also have a multibyte 
> extension that
> already implements multi-byte safe variants of common functions, why 
> make
> exception for fgetcsv() and add multibyte code into core?

I admit that few requests are in sight, but it's the case that
those who really want that support don't advocate it in English.

And I don't think fgetcsv() is an exception, since htmlentities() can
be referred to as an example that is placed in core and
supports multibyte strings. As I mentioned, purging that kind of
functionality into the mbstring extension doesn't solve the problem
in practice by any means.

>> 2) IMO speed is not a key factor here. People rather wants
>>     trust-worthy behaviour.
>
> When it's a few percent and the changes offer significant improvements 
> yes,
> but when were are talking about a performance loss of 250-300% or more 
> then
> performance must become a consideration as well.

If there are virtually no ways to improve it, it'd be natural to me
we dismiss the issue.

>> 3) fgetcsv() implementation in the stable branch is
>>     now too complicated to add a new feature to
>>     and also hard to maintain. We should be able to
>>     eliminate the mblen() calls for acceptable performance.
>>     See the attached result.
>
> What features are we talking about here? The only 2 features I can see 
> we may
> wish to add are >1 char long enclosures and separators and the binary 
> thing.
> Both of these features would be fairly trivial to add.

One thing I'm talking about here is escaping behaviour, which I 
mentioned
in the previous mail.

Moriyoshi