smarty icon indicating copy to clipboard operation
smarty copied to clipboard

Published 20 hours ago • verified publisher icon smarty-php

PHP8.2 compatibility

Open Progi1984 opened this issue 3 years ago • 12 comments

Added suport for PHP8.2 compatibility.

Thanks @ophian

Progi1984 avatar Jul 25 '22 11:07 Progi1984

@Progi1984 Please check modifier escape diff. I think mine at #767 looks different, see flags and additional parenthesis. Did you run tests on this?

ophian avatar Jul 25 '22 12:07 ophian

@ophian I fixed it and rebase the PR.

Progi1984 avatar Jul 27 '22 06:07 Progi1984

Please have a look at the unit tests? There are still failures.

wisskid avatar Aug 19 '22 07:08 wisskid

@wisskid I just fixed tests. Thanks image

Progi1984 avatar Aug 31 '22 07:08 Progi1984

Hi @wisskid, have you some time to restart the CI, please ?

Progi1984 avatar Sep 14 '22 17:09 Progi1984

CI is running. Can you explain the changes to the escape modifier compiler? It seems different in the way the methods are nested.

wisskid avatar Sep 14 '22 17:09 wisskid

@wisskid The CI is fixed.

I checked PHPUnit on PHP 8.2 RC / PHP 8.1 / PHP 8.0 / PHP 7.4 / PHP 7.3 / PHP 7.2 : https://github.com/Progi1984/smarty/actions/runs/3060762059

The encoding HTML-ENTITIES has been deprecated since PHP 8.2 : https://php.watch/versions/8.2/mbstring-qprint-base64-uuencode-html-entities-deprecated#html and tried to use the suggested replacement by php.watch. In this way, unit tests are not broken (Thanks to them :pray:)

Progi1984 avatar Sep 15 '22 13:09 Progi1984

Hi @Progi1984 Could you elaborate why this change was necessary? Was there an error you had? libs/sysplugins/smarty_internal_runtime_make_nocache.php

BTW. You were right adding the additional htmlspecialchars( to the escape modifier. I discovered that later also when I made some extending tests with escape etc. (I have some more on this to include) But I still think the ENT_COMPAT set should follow the PHP 8.2 change of giving it up in favour of the ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML40 triplet.

ophian avatar Sep 16 '22 08:09 ophian

@ophian Of course, in PHP 8.2, Smarty_Variable becomes \Smarty_Variable in var_export of the variable. So the regex didn't work.

I check the change (ENT_COMPAT => ENT_QUOTES | ENT_SUBSTITUTE | ENT_HTML401) in all versions and it's OK.

CI is :green_circle: : https://github.com/Progi1984/smarty/actions/runs/3080652601

Progi1984 avatar Sep 19 '22 07:09 Progi1984

Hi @wisskid, have you some time to restart the CI, please ?

Progi1984 avatar Sep 24 '22 10:09 Progi1984

Thanks for the CI @wisskid : it's :green_circle:

Progi1984 avatar Sep 28 '22 19:09 Progi1984

Hi @wisskid, have you some time to review this PR, please ? Thanks for your time.

Progi1984 avatar Oct 17 '22 15:10 Progi1984

@Progi1984 this looks great! I'll double check the encoding/escaping change and merge asap.

wisskid avatar Oct 17 '22 17:10 wisskid

@wisskid I would recommend to merge my PR https://github.com/smarty-php/smarty/pull/781 first, since that is the Smarty 4.2.0/1 cleanup as a base for the PHP 8.2 fixes.

ophian avatar Oct 18 '22 15:10 ophian

@wisskid I checked PHPUnit with : PHP 8.2 RC5 :heavy_check_mark: / PHP 8.1 :heavy_check_mark: / PHP 8.0 :heavy_check_mark: / PHP 7.4 :heavy_check_mark: / PHP 7.3 :heavy_check_mark: / PHP 7.2 :heavy_check_mark:

Sorry : I have removed your changes (but I have re-implemented in 4c39c54). And the PR has been rebased.

Progi1984 avatar Nov 18 '22 09:11 Progi1984

:tada: @wisskid (and thanks for discussions)

Progi1984 avatar Nov 22 '22 20:11 Progi1984