search

smarty-php / smarty

Smarty is a template engine for PHP, facilitating the separation of presentation (HTML/CSS) from application logic.
Other
2.24k stars 705 forks source link

Add PHP 8.4 support to Smarty #1043

Open Visualq opened 1 month ago

Visualq commented 1 month ago

This pull request updates the code base to support the deprecations introduced in PHP 8.4. It should be backwards compatible.

  1. Updated the function signatures that had implicit null assignment to typed parameters.
  2. Added PHP 8.4 to the 'run tests'-shell script.
  3. Bumped the PHP version in the README.md
scottchiefbaker commented 1 month ago

Wow... way to stay ahead of things. I support this effort in theory, but is the PHP 8.4 syntax 100% nailed down? Does it make sense to wait until we're 100% sure the syntax isn't gonna change.

I didn't even know PHP 8.4 alpha was out until I saw this issue :)

Visualq commented 1 month ago

Wow... way to stay ahead of things. I support this effort in theory, but is the PHP 8.4 syntax 100% nailed down? Does it make sense to wait until we're 100% sure the syntax isn't gonna change.

I didn't even know PHP 8.4 alpha was out until I saw this issue :)

I ran the test suite against the PHP 8.4 alpha version (no errors/or deprecation warnings). There has been a feature stop and it appears that a lot of 'proposed' deprecations for PHP 8.4 are still being discussed (https://wiki.php.net/rfc/deprecations_php_8_4). Functions that would require attention if all the deprecations are implemented/pass are:

The md5() and sha1() could be replaced with hash() however that would bump Smarty's minimum's PHP version to 8.0 or they could be polyfilled but this might be a performance debate. Maybe a simple function_exists could solve this without to much performance degredation

The strtok() is used once within Smarty and can be replaced.

The two deprecations regarding the error constants can easily be solved

This might not be a full PHP 8.4 support update for when it's released. The adjustments are safe down to PHP 7.1.

eileenmcnaughton commented 1 month ago

I took a look at this & support it being merged. I guess the only contentious part of merging it from my point of view is whether you want to declare php8.4 support - the other changes all seem to be fine as 'preparation' if you are not prepared to go all in