Everything

php

Bug 23654: str_replace doesn't handle multidimensional arrays

php.internals

Sara Golemon

23 years ago

I've got fixes for str_replace to let it handle multidimensional arrays, but
I'm not sure if it should be applied to the 4.3 branch or not.  On the one
hand it changes function behavior so it shouldn't be applied in stable.  On
the other hand, current behavior is ugly and wrong so it should be fixed..

Thoughts?

-Sara

Moriyoshi Koizumi

23 years ago

<pollita@php.net> wrote:

> I've got fixes for str_replace to let it handle multidimensional arrays, but
> I'm not sure if it should be applied to the 4.3 branch or not.  On the one
> hand it changes function behavior so it shouldn't be applied in stable.  On
> the other hand, current behavior is ugly and wrong so it should be fixed..
>
> Thoughts?

What is your fix like? Actually the current behaviour is ugly, but I don't
see the idea str_replace() should always be able to handle multi-dim
arrays as not all the users want it to do so. At most we'd be better off
if nested arrays were ignored.

Moriyoshi

Sara Golemon

23 years ago

> > I've got fixes for str_replace to let it handle multidimensional arrays,

but

> > I'm not sure if it should be applied to the 4.3 branch or not.
> >
> What is your fix like? Actually the current behaviour is ugly, but I don't
> see the idea str_replace() should always be able to handle multi-dim
> arrays as not all the users want it to do so. At most we'd be better off
> if nested arrays were ignored.
>

That may be the better way to go.  The fix I've got will traverse down the
array reentrantly, but it's occured to me that it'll choke on a recursive
array structure (i.e.:   $array[] = &$array; )  That's fixable too of
course, but it starts to get progressively ugly.  Simply patching in this
should do what you describe and keep everyone happy without making a
significant change in branch.  Maybe 5.0 can include an option to traverse
down, maybe not...

Index: ext/standard/string.c
===================================================================
RCS file: /repository/php4/ext/standard/string.c,v
retrieving revision 1.333.2.26
diff -u -r1.333.2.26 string.c
--- ext/standard/string.c       16 May 2003 06:21:39 -0000      1.333.2.26
+++ ext/standard/string.c       16 May 2003 17:52:01 -0000
@@ -2726,8 +2726,12 @@
/* For each subject entry, convert it to string, then
perform replacement
and add the result to the return_value array. */
while (zend_hash_get_current_data(Z_ARRVAL_PP(subject),
(void **)&subject_entry) == SUCCESS) {
-                       MAKE_STD_ZVAL(result);
-                       php_str_replace_in_subject(*search, *replace,
subject_entry, result);
+                       if (Z_TYPE_PP(subject_entry) != IS_ARRAY &&
Z_TYPE_PP(subject_entry) != IS_OBJECT) {
+                               MAKE_STD_ZVAL(result);
+                               php_str_replace_in_subject(*search,
*replace, subject_entry, result);
+                       } else {
+                               result = *subject_entry;
+                       }
/* Add to return array */
switch
(zend_hash_get_current_key_ex(Z_ARRVAL_PP(subject), &string_key,

&string_key_len, &num_key, 0, NULL)) {

Moriyoshi Koizumi

23 years ago

<pollita@php.net> wrote:

> > > I've got fixes for str_replace to let it handle multidimensional arrays,
> but
> > > I'm not sure if it should be applied to the 4.3 branch or not.
> > >
> > What is your fix like? Actually the current behaviour is ugly, but I don't
> > see the idea str_replace() should always be able to handle multi-dim
> > arrays as not all the users want it to do so. At most we'd be better off
> > if nested arrays were ignored.
> >
> That may be the better way to go.  The fix I've got will traverse down the
> array reentrantly, but it's occured to me that it'll choke on a recursive
> array structure (i.e.:   $array[] = &$array; )  That's fixable too of
> course, but it starts to get progressively ugly.  Simply patching in this
> should do what you describe and keep everyone happy without making a
> significant change in branch.  Maybe 5.0 can include an option to traverse
> down, maybe not...

Recursive arrays can be ignored because it is by no means a desired
behaviour IMO.

Anyway do we have to introduce str_replace_recursive() in 5.0 ? :) or most
likely the fifth argument?

Moriyoshi

Andrey Hristov

23 years ago

----- Original Message -----
From: "Moriyoshi Koizumi" <moriyoshi@at.wakwak.com>
To: <pollita@php.net>
Cc: <internals@lists.php.net>
Sent: Friday, May 16, 2003 9:22 PM
Subject: Re: [PHP-DEV] Bug 23654: str_replace doesn't handle
multidimensional arrays

> <pollita@php.net> wrote:
>
> > > > I've got fixes for str_replace to let it handle multidimensional

arrays,

> > but
> > > > I'm not sure if it should be applied to the 4.3 branch or not.
> > > >
> > > What is your fix like? Actually the current behaviour is ugly, but I

don't

> > > see the idea str_replace() should always be able to handle multi-dim
> > > arrays as not all the users want it to do so. At most we'd be better

off

> > > if nested arrays were ignored.
> > >
> > That may be the better way to go.  The fix I've got will traverse down

the

> > array reentrantly, but it's occured to me that it'll choke on a

recursive

> > array structure (i.e.:   $array[] = &$array; )  That's fixable too of
> > course, but it starts to get progressively ugly.  Simply patching in

this

> > should do what you describe and keep everyone happy without making a
> > significant change in branch.  Maybe 5.0 can include an option to

traverse

> > down, maybe not...
>
> Recursive arrays can be ignored because it is by no means a desired
> behaviour IMO.
>
> Anyway do we have to introduce str_replace_recursive() in 5.0 ? :) or most
> likely the fifth argument?

Little problem is that the fourth parameter should be real var (as far as I
can see),
and ppl who want to use the recursive behaviour should pass a var.
Maybe if the fourth argument is not real var then use it as bool for the
recursive stuff.
On the other hand if it is real var then check for the 5th parameter. Too
confusing, isn't it :)

Andrey

Sara Golemon

23 years ago

We can actually move $count out to the fifth parameter and put recursive as
a boolean parameter in position four since $count is a new feature in PHP5
so there's no BC issue at stake.

Anyway, if I don't get a complaint in the next six hours I'll go ahead and
commit that fix in branch and head to simply ignore subarrays and pass them
unchanged to the result.

> > Anyway do we have to introduce str_replace_recursive() in 5.0 ? :) or

most

> > likely the fifth argument?
>
> Little problem is that the fourth parameter should be real var (as far as

I

Andrey Hristov

23 years ago

---- Original Message -----
From: "Moriyoshi Koizumi" <moriyoshi@at.wakwak.com>
To: <pollita@php.net>
Cc: <internals@lists.php.net>
Sent: Friday, May 16, 2003 9:22 PM
Subject: Re: [PHP-DEV] Bug 23654: str_replace doesn't handle
multidimensional arrays

>
> Recursive arrays can be ignored because it is by no means a desired
> behaviour IMO.
>
> Anyway do we have to introduce str_replace_recursive() in 5.0 ? :) or most
> likely the fifth argument?
>

one different way is intoductin str_replace_recursive()

Andrey

Derick Rethans

23 years ago

On Sat, 17 May 2003, Moriyoshi Koizumi wrote:

> <pollita@php.net> wrote:
>
> > I've got fixes for str_replace to let it handle multidimensional arrays, but
> > I'm not sure if it should be applied to the 4.3 branch or not.  On the one
> > hand it changes function behavior so it shouldn't be applied in stable.  On
> > the other hand, current behavior is ugly and wrong so it should be fixed..
> >
> > Thoughts?
>
> What is your fix like? Actually the current behaviour is ugly, but I don't
> see the idea str_replace() should always be able to handle multi-dim
> arrays as not all the users want it to do so. At most we'd be better off
> if nested arrays were ignored.

Agreed on that. Just make it skip arrays for replacements.

Derick

--
"my other box is your windows PC"
-------------------------------------------------------------------------
Derick Rethans                                 http://derickrethans.nl/
PHP Magazine - PHP Magazine for Professionals       http://php-mag.net/
-------------------------------------------------------------------------