Everything

php

fgetcsv() conclusion

php.internals

Ilia A.

22 years ago

It's been a few days since the last comments regarding the fgetcsv() in PHP
5.0 and I think it would be an opportune time to decide on a solution for the
problem. Based on the various commentary we appear to have 3 alternatives
that for better or worse would resolve the situation.

1) Leave things they way they are now, giving everyone a partial support for
multibyte string in fgetcsv() and 2-3 performance decrease in all instances.

2) Backport fgetcsv() from 4.3.X tree, giving everyone a fast variant of the
function and introduce a fully multibyte mb_fgetcsv() into mbstring extension
based on the current fgetcsv() in PHP 5.0.

3) Put the php_mblen macro in php_string.h inside ifdef HAVE_MBSTRING, which
would disable multibyte functionality of fgetcsv() for people without
mbstring support in their installation. This would just about restore
performance and at the same time leave multibyte support for people who need
it (use mbstring extension).


My personal preference is option #2.

Ilia

Moriyoshi Koizumi

22 years ago

On 2003/12/22, at 3:12, Ilia Alshanetsky wrote:

> 1) Leave things they way they are now, giving everyone a partial
> support for
> multibyte string in fgetcsv() and 2-3 performance decrease in all
> instances.

Of course this is my choice.

> 3) Put the php_mblen macro in php_string.h inside ifdef HAVE_MBSTRING,
> which
> would disable multibyte functionality of fgetcsv() for people without
> mbstring support in their installation. This would just about restore
> performance and at the same time leave multibyte support for people
> who need
> it (use mbstring extension).

Why should we set up a relation to only mbstring extension here? I think
HAVE_ICONV should be take into consideration as well in case we choose
this option after all.

Regards,
Moriyoshi

Christian Schneider

22 years ago

Moriyoshi Koizumi wrote:

>> 1) Leave things they way they are now, giving everyone a partial
>> support for
>> multibyte string in fgetcsv() and 2-3 performance decrease in all
>> instances.
>
> Of course this is my choice.

I'm not sure this is what you want because the words "_partial_ support"
seem to be the catch here. I guess Ilia argues that the current way is
broken.

- Chris

Moriyoshi Koizumi

22 years ago

Christian Schneider <cschneid@cschneid.com> wrote:

> Moriyoshi Koizumi wrote:
> >> 1) Leave things they way they are now, giving everyone a partial
> >> support for
> >> multibyte string in fgetcsv() and 2-3 performance decrease in all
> >> instances.
> >
> > Of course this is my choice.
>
> I'm not sure this is what you want because the words "_partial_ support"
> seem to be the catch here. I guess Ilia argues that the current way is
> broken.

What Ilia is mentioning as "partial support" is that the current
implementation can not handle multibyte delimiters and enclosures,
which I don't find necessary. I've never encountered a real situation
that requires multibyte delimiters or enclosures. Did you read through
the entire thread..?

Moriyoshi

David Giffin

22 years ago

Hi All,

I wanted to check if something changed in the preg_replace_callback()
function. After going to beta3 we are now not able to make static
object/method calls, that were working on beta2. Here is the error
message:

Warning: preg_replace_callback(): requires argument 2,
'self::replacePropertyCallback', to be a valid callback in
/usr/local/phing/classes/phing/parser/ProjectConfigurator.php on line 202

Thanks,
David

Stefan Walk

22 years ago

On Mon, Dec 22, 2003 at 05:32:53AM -0800, David Giffin wrote:

> I wanted to check if something changed in the preg_replace_callback()
> function. After going to beta3 we are now not able to make static
> object/method calls, that were working on beta2. Here is the error
> message:
>
> Warning: preg_replace_callback(): requires argument 2,
> 'self::replacePropertyCallback', to be a valid callback in
> /usr/local/phing/classes/phing/parser/ProjectConfigurator.php on line 202

Hi, using
preg_replace_callback($pattern, array($classname, $methodname), $subject)
works here in b3. That's the general notation for static method calls in
callbacks, afaik.

--
Regards,
Stefan Walk
<swalk@prp.physik.tu-darmstadt.de>

Hans Lellelid

22 years ago

> On Mon, Dec 22, 2003 at 05:32:53AM -0800, David Giffin wrote:
>> I wanted to check if something changed in the preg_replace_callback()
>> function. After going to beta3 we are now not able to make static
>> object/method calls, that were working on beta2. Here is the error
>> message:
>>
>> Warning: preg_replace_callback(): requires argument 2,
>> 'self::replacePropertyCallback', to be a valid callback in
>> /usr/local/phing/classes/phing/parser/ProjectConfigurator.php on line
>> 202
>
> Hi, using
> preg_replace_callback($pattern, array($classname, $methodname), $subject)
> works here in b3. That's the general notation for static method calls in
> callbacks, afaik.
>

The call that is causing problems is a static call.  Is this no longer
possible?  This is the line that is generating the above error:

$sb = preg_replace_callback('/\$\{([^}]+)\}/', array('self',
'replacePropertyCallback'), $value);

This was AFAIK the way to invoke methods in a static class.  Perhaps we
now need to specify classname rather than 'self'.

Thanks,
Hans

Stefan Walk

22 years ago

On Mon, Dec 22, 2003 at 09:34:38AM -0500, Hans Lellelid wrote:

> This was AFAIK the way to invoke methods in a static class.  Perhaps we
> now need to specify classname rather than 'self'.

Just use __CLASS__.

--
Regards,
Stefan Walk
<swalk@prp.physik.tu-darmstadt.de>

Hans Lellelid

22 years ago

Hi -

We're using the Reflection API in a project for which I'm lead developer.
Seems to be something changed between beta-2 and beta-3.  We're now
getting the following exception when calling getClass() on a class hint:

exception 'reflection_exception' with message 'Class FileSet does not
exist' in C:\sandbox\phing\classes\phing\IntrospectionHelper.php:244

The class does exist -- i.e. if I dump get_declared_classes() right before
the error line I do indeed see FileSet (well, 'fileset', of course).

The code that triggers this reflection_exception looks like this:

if (($hint = $params[0]->getClass()) === null ) {
throw new
BuildException($method->getDeclaringClass()->getName()."::".$method->getName()."()
method MUST use a class hint to indicate the class type of parameter.");
}

Has something changed? The Reflection_Parameter class looks the same
(API-wise), but I don't know if I'm doing something wrong here or of this
is a legitimate bug.

Thanks,
Hans

Timm Friebe

22 years ago

On Mon, 2003-12-22 at 16:05, Hans Lellelid wrote:

> Hi -
>
> We're using the Reflection API in a project for which I'm lead developer.
> Seems to be something changed between beta-2 and beta-3.  We're now
> getting the following exception when calling getClass() on a class hint:

[...]

Works fine here (see below), care to provide a small reproducing script?

thekid@friebes:~ > cat reflection_parameter_test.php
<?php
class FileSet {
public function test($set) { }
}

$method= new Reflection_Method('FileSet', 'test');
$params= $method->getParameters();

var_dump(
$params[0]->getClass(),
$method->getName(),
$method->getDeclaringClass()->getName()
);

if (NULL === ($hint= $params[0]->getClass())) {
throw new Exception(sprintf(
'%s::%s() method MUST use a class hint...',
$method->getDeclaringClass()->getName(),
$method->getName()
));
}
?>

thekid@friebes:~ > php-dev reflection_parameter_test.php
NULL
string(4) "test"
string(7) "FileSet"

Fatal error: Uncaught exception 'exception' with message
'FileSet::test() method MUST use a class hint...' in
/usr/home/thekid/reflection_parameter_test.php:16
Stack trace:
#0 {main}
thrown in /usr/home/thekid/reflection_parameter_test.php on line 16

thekid@friebes:~ > php-dev -v
PHP 5.0.0RC1-dev (cli) (built: Dec 23 2003 20:51:13) (DEBUG)
Copyright (c) 1997-2003 The PHP Group
Zend Engine v2.0.0-dev, Copyright (c) 1998-2003 Zend Technologies

- Timm