phpspec/php-diff deprecated exception

[crtlib]

When using Gii with php 5.5 I'm getting exception that using preg_replace with '/e' modifier is deprecated in php-diff/lib/Diff/Renderer/Html/Array.php:180. Here is a fix:

        $line = preg_replace_callback('# ( +)|^ #', function ($m) {
                return $this->fixSpaces(isset($m[1]) ? $m[1] : '');
            }, $line);

And my question is - should we fix it at phpspec/php-diff or should we fork it and do it ourselves? Sorry for noob question - little experience with github)))

[cebe]

We should create a pull request for phpspec/php-diff and require the fixed version in composer.

[cebe]

Well seems like the repo is not maintained very well. in the original repo https://github.com/chrisboulton/php-diff there is alrey a pull request regarding php 5.5 compatibility open for long time. Looks like it would be best to create our own fork and maintain these things on our own.

[samdark]

Worth trying to contact maintainer via email or IM.

[crtlib]

everzet? you're his habramate, you're closer to him to do it)

[samdark]

@crtlib he's not code maintainer.

I'll handle the conversation.

[samdark]

Nothing from original author so far. @crtlib seems to be right about the fact that composer package points to a fork https://github.com/phpspec/php-diff/ Will ask everzet then.

[samdark]

phpspec/php-diff#2

[crtlib]

I think it's a wrong commit. The callback should accept parameter $matches. If you insist on avoiding anonymous function then we should modify fixSpaces:

public static function fixSpaces($matches)
{
    $spaces = isset($matches[1]) ? $matches[1] : '';
    $count = strlen($spaces);
    if($count == 0) {

But still I think that it'd be better to use backreference as close as possible to the regular expression itself. Just as in my snippet at the beginning.

[cebe]

Please move discussion about the actual pull request to there so maintainer can see it and take part in discussion.

[samdark]

@crtlib fixed. Thanks. The usage of static method is not to bump library requirements to 5.3. If there are more to discuss, let's do it in that pull request as @cebe suggested.

[crtlib]

Now I see. Thanks for explaining. I have nothing more to add)))

[samdark]

Changes merged to phpspec repo. Closing.