punkave/symfony1

Deprecation notice for preg_replace() with /e modifier

Closed this issue · 7 comments

From the command line, when I type
$ ./symfony help
I get the following :

Deprecated: preg_replace(): The /e modifier is deprecated, use preg_replace_callback instead in /home/symfony-punkave/lib/vendor/symfony/lib/task/sfTask.class.php on line 307
Usage:
 symfony help [--xml] [task_name]
...

I use PHP 5.6.24 on Debian Wheezy.

So the error is obviously caused by the deprecation of the /e modifier for preg_replace in PHP 5.5, as stated in the PHP doc.

I solved it by changing the incriminated line from

return preg_replace('/\[(.+?)\|(\w+)\]/se', '$this->formatter->format("$1", "$2")', $this->detailedDescription);

to

preg_replace_callback('/\[(.+?)\|(\w+)\]/s', function($m) { return $this->formatter->format($m[1], $m[2]); }, $this->detailedDescription);

to use the recommended preg_replace_callback function, and the error disappeared. But I'm not sure I did it well so I prefer not to submit a pull request before discussing it.

Looks right to me (and apparently the task now runs for you). I'd take that
PR. It should be bc as far back as we need.

On Thu, Aug 4, 2016 at 10:05 AM, Sylvain Lelièvre notifications@github.com
wrote:

From the command line, when I type
$ ./symfony help
I get the following :

Deprecated: preg_replace(): The /e modifier is deprecated, use preg_replace_callback instead in /home/symfony-punkave/lib/vendor/symfony/lib/task/sfTask.class.php on line 307
Usage:
symfony help [--xml] [task_name]
...

I use PHP 5.6.24 on Debian Wheezy.

So the error is obviously caused by the deprecation of the /e modifier
for preg_replace in PHP 5.5, as stated in the PHP doc
http://php.net/manual/en/function.preg-replace.php.

I solved it by changing the incriminated line from

return preg_replace('/[(.+?)|(\w+)]/se', '$this->formatter->format("$1", "$2")', $this->detailedDescription);

to

preg_replace_callback('/[(.+?)|(\w+)]/s', function($m) { return $this->formatter->format($m[1], $m[2]); }, $this->detailedDescription);

to use the recommended preg_replace_callback function, and the error
disappeared. But I'm not sure I did it well so I prefer not to submit a
pull request before discussing it.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#5, or mute the thread
https://github.com/notifications/unsubscribe-auth/AAB9fWEWNBQ7b4f0G9xLnFaB8XXMrWsgks5qcfGogaJpZM4Jcuzh
.

*THOMAS BOUTELL, *SUPPORT LEAD
P'UNK AVENUE | (215) 755-1330 | punkave.com

This would be great. In the past I've stuck with PHP 5.3 and 5.4 because of deprecation warnings but we're planning on switching soon to 5.6 so I'm glad for this issue to be resolved.

Sorry but I screwed my PR #6 : I forgot to remove the /e modifier from the new function call... I'm on it.

PR #7 is waiting for merge. Now it should be ok. Sorry again.

I made some more research and it seems that my patch could introduce BC break with PHP 5.3.

Here is my code (in /lib/task/sfTask.class.php):

public function getDetailedDescription()
{
    return preg_replace_callback('/\[(.+?)\|(\w+)\]/s', function($m) { return $this->formatter->format($m[1], $m[2]); }, $this->detailedDescription);
}

The problem is my use of $this in a closure, which is allowed since PHP 5.4 only (see the Changelog section of the Anonymous functions man page of PHP).

I had a look at the LExpress fork of Symfony1 and I prefer the way they handled it, by using an intermediary variable to avoid the use of $this within the closure :

public function getDetailedDescription()
{
    $formatter = $this->getFormatter();
    return preg_replace_callback('/\[(.+?)\|(\w+)\]/s', function ($match) use ($formatter) {
        return $formatter->format($match['1'], $match['2']);
    }, $this->detailedDescription);
}

@boutell Do I have to close the issue by myself ? I don't know if there are conventions about who close a ticket when the merge is done.

It's closed.

On Tue, Aug 16, 2016 at 5:14 AM, Sylvain Lelièvre notifications@github.com
wrote:

@boutell https://github.com/boutell Do I have to close the issue by
myself ? I don't know if there are conventions about who close a ticket
when the merge is done.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#5 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAB9ffdA_MgLs0Hjv5XH-PGlj3xB3jA-ks5qgX9bgaJpZM4Jcuzh
.

*THOMAS BOUTELL, *SUPPORT LEAD
P'UNK AVENUE | (215) 755-1330 | punkave.com