PHP 5.5 / nl2br plugin / preg_replace() /e modifier

[hanno]

It seems even with mysqli enabled there are still deprecation issues in serendipity (version 1.7.2) with php 5.5.
I get this warning:
preg_replace(): The /e modifier is deprecated, use preg_replace_callback instead in /home/hanno/websites/blog.hboeck.de/htdocs/plugins/serendipity_event_nl2br/serendipity_event_nl2br.php on line 156

This is the line in question:
return preg_replace('~\001(\d+)~e', '$_buf[$1]', $text);

[Timbalu]

Hmm, how bad - sorry, I had a search run for this, which I must have overseen in the result set.
Could you please give this a try for a test... (untested)

Code: Select all

return preg_replace_callback('!\001(\d+)!', create_function ('$_buf[$1]', 'return $_buff[$1];'), $text);

Unless this works, you can also set serendipity_config.inc.php ~Line 57 to:

Code: Select all

    error_reporting(E_ALL & ~(E_STRICT|E_NOTICE|E_DEPRECATED)); 

[Timbalu]

Since there was no answer on this yet, I had some tests on my own using this suggestion of mine on PHP 5.4.x and worked around some issues.

So the "final" solution seems to be like this, replacing that Line 156, in serendipity_event_nl2br.php.

Code: Select all

        // As of PHP 5.5 deprecated /e modifier we define our callback here and import $_buf into its scope ...
        $callback = function ($matches) use ($_buf) { return $_buf[$matches[1]]; };
        return preg_replace_callback('!\001(\d+)!', $callback, $text);

Please, anybody with PHP 5.5.x on Board, check this callback to work on all isolating and restoring array values. This can be tested with either the nl2p or the noBr isolations.

I'll wait some time for anybody wanting to check this to work with all PHP Versions since the 5.2 Series, before I commit this into core. The bad thing about using closures (anonymous functions) like is, that is relies on PHP >= 5.3.0.

If anybody wants to help making this as stable as possible, please run intensive checks, if using a Serendipity-Blog, taking text entries, saving or output, via the NL2BR-Plugin-Options: NL2P or ISOBR, since this is a very sensitive and important feature for the correct isolation/restore replacement of markup. Tests would need more than one isolating block.

[Timbalu]

Hm..., I still need some (more) testers on this, please!

[ICE]

Sorry, but my Server has only PHP 5.3.18 ... i can't test it for you

[Timbalu]

Thats why I also wrote: "...wanting to check this to work with all PHP Versions since the 5.2 Series, before I commit..." since I would only workaround for User with 5.2.x

[mattsches]

I just compiled PHP 5.5.1 .. will try to test your code tomorrow. Stay tuned!

[mattsches]

Ok, I tested it, the deprecation warning is gone However, I'm not sure your code does what it's supposed to do because I simply do not understand what the plugin does (i.e. I'm too lazy/busy to read and comprehend all the code, sorry).

If you can give me any examples of what both $text and $buf may contain, I will run it with "real" values

IMHO, you don't have to use a closure here. You can also add a callback method to the serendipity_event_nl2br class and call it like this

Code: Select all

return preg_replace_callback('!\001(\d+)!', array($this, 'callbackMethod'), $text);

[Timbalu]

Well, I know its working, since I have that on my 5.4 dev blog without any errors or unwanted replacement yet. But what I need, are some more intensive tests with different PHP versions, and with anykind of structured markup, to see the isolation/restore feature work as before. Thanks for anyone tanking time!

Simple examples for noBr (<nl></nl>) usage:

Code: Select all

blah bla without nobr 
yes without
for sure

<nl>
    <div>
        <div>I am the second $buf</div>
        <div>and I am not going to move</div>
    </div>
</nl>

And here is number 3
which follows...

<nl>
    <blockquote>
        <ol>
            <li>never</li>
            <li>say never</li>
            <li>again</li>
        </ol>
    </blockquote>
</nl>

<nl>
<table border="1" cellpadding="1" cellspacing="1" style="width:500px">
    <tbody>
        <tr>
            <td>test 1</td>
            <td>&nbsp;1.2</td>
        </tr>
        <tr>
            <td>test 2</td>
            <td>&nbsp;2.2</td>
        </tr>
        <tr>
            <td>test 3</td>
            <td>&nbsp;3.2</td>
        </tr>
    </tbody>
</table>
</nl>

Things like this and even more extended ones and also similar tests with NL2P.
Check on how these saved entries will look like in the frontend, being processed by nl2br/nl2p.

Using that closure or a callback method in class are roughly the same. Would that benefit to change?

[mattsches]

Problem is, I have multiple versions of PHP running and I can switch between them using phpenv. But I haven't managed to make this work with Apache. That's why I cannot test it in a browser, but only using the shell.

Unit tests would be great in this case If I find the time, I will go ahead and write them.

I think, you can use preg_replace_callback() with an object method call even in versions >5 && <5.3.0. That's the main difference, I think.

[Timbalu]

Yes sure, unit test would be great to have here. I never really understood why you didn't get any feedback on your "unit tests for plugins" thread.

Do you mean somehow like this (to keep working with all PHP versions since 5.2.3)?

Code: Select all

return preg_replace_callback('!\001(\d+)!', array($this, 'serendipity_event_nl2br::mycallbackMethod'), $text);

How would you set the static function then? Or would that be equal to https://github.com/s9y/Serendipity/blob ... r.php#L148 (...and that said...: will this not fail on 5.2 too)?

I for myself did find this much easier, keeping sort of a "(readable) history":

Code: Select all

    function restore($text) {
        global $_buf;
        if (version_compare(PHP_VERSION, '5.3.0') >= 0) {
            // As of PHP 5.5 deprecated /e modifier we define our callback here and import $_buf into its scope ...
            $callback = function ($matches) use ($_buf) { return $_buf[$matches[1]]; };
            return preg_replace_callback('!\001(\d+)!', $callback, $text);
        } else {
            return preg_replace('~\001(\d+)~e', '$_buf[$1]', $text);
        }
    }

[mattsches]

Yes sure, unit test would be great to have here. I never really understood why you didn't get any feedback on your "unit tests for plugins" thread.

Me neither. This here is a perfect example of why unit tests are useful, I guess… Thanks for the feedback

Do you mean somehow like this (to keep working with all PHP versions since 5.2.3)?

Code: Select all

return preg_replace_callback('!\001(\d+)!', array($this, 'serendipity_event_nl2br::mycallbackMethod'), $text);

How would you set the static function then? Or would that be equal to https://github.com/s9y/Serendipity/blob ... r.php#L148 (...and that said...: will this not fail on 5.2 too)?

I would prefer the syntax like in the isolate() method and wouldn't go for the static function (I don't like static functions too much). Anyway, in this case it doesn't matter, I suppose.

I for myself did find this much easier, keeping sort of a "(readable) history":

Code: Select all

    function restore($text) {
        global $_buf;
        if (version_compare(PHP_VERSION, '5.3.0') >= 0) {
            // As of PHP 5.5 deprecated /e modifier we define our callback here and import $_buf into its scope ...
            $callback = function ($matches) use ($_buf) { return $_buf[$matches[1]]; };
            return preg_replace_callback('!\001(\d+)!', $callback, $text);
        } else {
            return preg_replace('~\001(\d+)~e', '$_buf[$1]', $text);
        }
    }

Ideally, we should try to find a way that will make sense in the future without breaking stuff from the (PHP5+) past (I'm not talking of PHP4, we can ignore that). So if preg_replace_callback() works with all versions PHP5 and up, use it. For keeping our history, we have git

[Timbalu]

This was fixed again due to a bug with PHP < 5.3 versions.
Better now use

Code: Select all

    function restore_callback($matches) {
        global $_buf;
        return $_buf[$matches[1]];
    }

    function restore($text) {
        return preg_replace_callback('!\001(\d+)!', array($this, 'restore_callback'), $text);
    }

on fresh installs - or download 1.7.4(-beta).