Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PHP7 Compatibility Issues #253

Closed
r-a-y opened this issue Jan 5, 2017 · 4 comments
Closed

PHP7 Compatibility Issues #253

r-a-y opened this issue Jan 5, 2017 · 4 comments

Comments

@r-a-y
Copy link
Member

r-a-y commented Jan 5, 2017

As raised on the cbox.org forums, there are some PHP7 compatibility issues.

I've run cbox-theme through php7cc (stable version) and these are the errors that need to be fixed up:

File: wp-content\themes\cbox-theme\engine\ICE\lib\markdown\markdown.php             
> Line 218: PHP 4 constructors are now deprecated                                                                 
    function Markdown_Parser()                                                                                    
    {                                                                                                             
    }                                                                                                             
                                                                                                                  
                                                                                                                  
File: wp-content\themes\cbox-theme\engine\ICE\lib\textile\classTextile.php          
> Line 364: PHP 4 constructors are now deprecated                                                                 
    function Textile()                                                                                            
    {                                                                                                             
    }                                                                                                             
> Line 617: Removed function "split" called                                                                       
    split(';', $s);                                                                                               
> Line 1531: Removed regular expression modifier "e" used                                                         
    preg_replace('/(?<=\\S)\\[([0-9]+)([\\!]?)\\](\\s)?/Ue', '$this->footnoteID(\'\\1\',\'\\2\',\'\\3\')', $text);

Warnings:

File: wp-content\themes\cbox-theme\engine\api\screens.php       
> Line 172: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();                                                                          
                                                                                              
                                                                                              
File: wp-content\themes\cbox-theme\engine\ICE\base\renderer.php 
> Line 164: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();                                                                          
> Line 183: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();                                                                          
> Line 215: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();                                                                          
> Line 226: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();                                                                          
> Line 237: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();                                                                          
                                                                                              
                                                                                              
File: wp-content\themes\cbox-theme\engine\ICE\dom\element.php   
> Line 236: Function argument(s) returned by "func_get_args" might have been modified         
    func_get_args();


File: wp-content\themes\cbox-theme\engine\ICE\lib\markdown\markdown.php
> Line 360: Possible array element creation during by-reference assignment                           
    $this->titles[$link_id] =& $matches[4];                                                                                                                


File: wp-content\themes\cbox-theme\engine\ICE\schemes\scheme.php 
> Line 961: Function argument(s) returned by "func_get_args" might have been modified          
    func_get_args();                                                                           
boonebgorges added a commit that referenced this issue Mar 22, 2017
@boonebgorges
Copy link
Member

Thanks, @r-a-y. I've fixed three of the four errors. The third one will be harder:

Line 1531: Removed regular expression modifier "e" used

I think that fixing this will require rewriting as preg_replace_callback(). Do we have any sense of where/when this function might be called? What is the purpose of the Textile parser?

@r-a-y
Copy link
Member Author

r-a-y commented Mar 22, 2017

Thanks for looking into this, @boonebgorges.

I think the Textile code has something to do with the Markdown parser used on the "Dev Docs" section of the "Appearance > CBOX Theme Options" admin page.

boonebgorges added a commit that referenced this issue Mar 28, 2017
@boonebgorges
Copy link
Member

In case anyone ever wants to verify 6b98ee3, here were my two testcases:

require_once 'wp-content/themes/cbox-theme/engine/ICE/lib/textile/classTextile.php';

$t = new Textile();
$text = 'footnote foo[10] bar';
$found = $t->footnoteRef( $text );
var_Dump( $found );

$text = 'footnote foo[10!] bar';
$found = $t->footnoteRef( $text );
var_Dump( $found );

@boonebgorges
Copy link
Member

I've reviewed the warnings and I think we can probably safely avoid them. They're all related to reference-vs-value issues, and while there's no way I can audit the whole codebase to ensure that we're not modifying items that are being returned by reference, the risk in these specific cases seems pretty low when compared to the amount of refactoring that'd have to take place to remove them.

Going to close as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants