smarty-php / smarty

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

Some php function have error with registerPlugin #961

Open Nightprince opened 7 months ago

Nightprince commented 7 months ago

reset(), key(), end() , ... have error when we use registerPlugin, error :

Error: ErrorException: reset(): Argument #1 ($array) must be passed by reference, value given in F:\laragon\www\CMS\vendor\smarty\smarty\src\Extension\CallbackWrapper.php:29

$this->registerPlugin(Smarty::PLUGIN_MODIFIER, 'end', 'end'); $this->registerPlugin(Smarty::PLUGIN_MODIFIER, 'key', 'key'); $this->registerPlugin(Smarty::PLUGIN_MODIFIER, 'reset', 'reset'); ...

wisskid commented 7 months ago

I don't think there's an easy fix for this. Can you change your template to use a foreach loop?

Nightprince commented 7 months ago

No, I don't need foreach.

Nightprince commented 7 months ago

What is advantage of using version 5 when all default functions of PHP are removed and must be added manually and there are problems with some functions.

wisskid commented 7 months ago

Smarty 5 adds support for ternary operator and the null coalescing operator, positional parameters for custom tags and a whole new extension architecture.

Nightprince commented 7 months ago

Well, they are useful if smarty has no problem with default PHP functions. In version 4, many people may have used many PHP functions in smarty that do not have their list. Finding these functions and also having problems in calling them is a very difficult issue.

Nightprince commented 7 months ago

I have listed some of functions:

wisskid commented 7 months ago

@Nightprince for a discussion about the decision to drop support for the use of unregistered callables in v5, see https://github.com/smarty-php/smarty/discussions/967. For the breaking change regarding passing variables by reference, I'll use https://github.com/smarty-php/smarty/issues/964 and close this issue.

Nightprince commented 7 months ago

@Nightprince for a discussion about the decision to drop support for the use of unregistered callables in v5, see #967. For the breaking change regarding passing variables by reference, I'll use #964 and close this issue.

Why should issue be closed when problem is not fixed?

wisskid commented 7 months ago

So we don't have 2 open issues for the same problem.

wisskid commented 7 months ago

Re-opening this, since it seems #964 is fixed (for Smarty > v4), but we cannot port that fix to Smarty v5.

Nightprince commented 7 months ago

I suggest you add these as default in Smarty.

Namidas commented 1 month ago

It is indeed an issue that any programmer working extensively with call_user_func_array will run into sooner or later...on the other hand, you shouldn't have all that programming/logic in your templates (thus, as suggested, a foreach block should suffice).

I found two possible solutions, but they depend on each specific scenario, you either move all that logic outside of your template where you do something with your data, prepare it in a way, whatever...or you move all that logic into a single modifier/function, and you do that in plain PHP (outside of the smarty sintax/engine/etc).

The other solution is even more case-specific, but essentially, you wrap the problematic function into another one that doesn't use references but returns the modified variable, sort of like chainning, essentialy.

For instance, with reset:

$this->registerPlugin(Smarty::PLUGIN_MODIFIER, 'reset', function($array) { $res = reset($array); return [$array,$res]; });

Notice how this example returns the modified array and the original result of 'reset', both as part of a single array, so you won't be able to "translate" PHP into Smarty as-is, you would still need to change your logic a little.

Again, it all depends on each specific scenario, but, "again", you shouldn't have this level of logic inside a template (which should be focused on presentation and no deeper-logic than a simple loop through data).


On a totally different note...I have no idea how this works internally (and a quick look through the code got me nowhere), but this problem arises on run-time, that is, after compile, that is, after PHP code has been written programatically...

A simple fix could be changing how the function handle() is being called, call it with a reference array of all the arguments instead of the arguments themselves, so instead of compiling something like:

$foo->handle($arg1,$arg2);

compile something like:

$args = [$arg1,$arg2]; $foo->handle($args);

and then change the definition of CallbackWrapper->handle(...$params), to CallbackWrapper->handle(&$params) and it should work smooth as silk with call_user_func_array.


Found the correct file, quick edit, quick test, and worked wonderfully.

Specific to modifiers (potentially you may need to change this anywhere calling CallbackWrapper->handle), on file: compile/ModifierCompiler.php, #60, you have:

$output = sprintf( '$_smarty_tpl->getSmarty()->getModifierCallback(%s)(%s)', var_export($modifier, true), $params );

Just change it to:

$output = "''; \$fooParams = Array({$params});\necho "; $output .= sprintf( '$_smarty_tpl->getSmarty()->getModifierCallback(%s)($fooParams)', var_export($modifier, true) );

And then change the CallbackWrapper->handle definition from ...$params to &$params.

This is a very rudimentary fix, I couldn't find where the "original" echo was coming from, that's why I start the fix with an empty string (for that "floating" echo), and for the same reason I end up the changes with a new echo. It's ugly, but it works, with deep knowledge of smarty's source code it can be done the proper way.

On the other hand, there is a real issue with this: it only works as long as that modifier is not nested into anything else (for instance an array definition), otherwise you end up with invalid PHP (as this $fooParams ends up being defined inside an array definition).

In conclusion, even though the solution is quite simple, nesting implies that you would need to pre-parse everything to get all the definitions of these params arrays and compile/write that before compiling/writing the code that uses them in any nested fashion...

...but, hey! it works ♥


One last edit, I would still like to close this very emphatically insisting on the fact that you shouldn't have deep logic on your templates, just presentation, and anything using references usually changes data with more logic-related reasons rather than presentation-related.