smarty-php / smarty

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

Wrong implode/join use? #981

Closed Ninjinka closed 7 months ago

Ninjinka commented 7 months ago

Hello as php.net state implode (or join since it's an alias) should be

implode([string] $separator, [array] $array): [string]

Why smarty send a deprecation notice if used correctly? also the deprecation suggest to use join and with as parameters first array than separator (as in legacy mode for php 7.4)

Is not best to be php compilant?

scottchiefbaker commented 7 months ago

@Ninjinka can you post an example test case of how you're calling this?

scottchiefbaker commented 7 months ago
// https://github.com/smarty-php/smarty/issues/561
function smarty_modifier_join($array, $glue = '') {
    return join($glue, $array);
}

That is my join modifier which works without warnings.

wisskid commented 7 months ago

Is not best to be php compilant?

Smarty is Smarty and PHP is PHP. Smarty has no intention to mimic the syntax of PHP.

In my opinion, using implode as follows looks rather funny:

{","|implode:$array} {* <- this is weird *}

And this is much better:

{$array|join:","}
j-applese3d commented 1 month ago

Smarty is Smarty and PHP is PHP.

True. I agree that join and split should flip the argument order.

However, if I want to register implode and use it in the PHP-way, then why have a deprecation notice?

In my case:

// register the function:
$smarty->registerPlugin('modifier', 'implode', 'implode');

// and in my template, use implode just like I do in PHP:
<input type="text" value="{implode(',', $ids)}">

With Smarty v5.4.1 this will give me a deprecation notice:

Using implode is deprecated. Use join using the array first, separator second.

I understand that I can use {$ids|join:','} instead — but I think that I should not be warned against using a specific PHP-land function that I explicitly want and have registered.

Since implode isn't in the list of modifiers in the v5 documentation, can it be treated as if it were a normal user-created modifier?

wisskid commented 1 month ago

Hmmm I think the (undocumented) internal implode handler takes precedence.

Maybe we can move it to a special extension for deprecated /undocumented stuff that is loaded last in line. That way, any user defined modifier could take precedence.

j-applese3d commented 1 month ago

Right. If the order of the extension handlers is changed in Smarty->__construct(), the user defined modifier will be preferred.

I guess the question becomes: should all user-defined extension handlers take priority?

Core -> BC -> Default


EDIT: oh, I think I understand better now. You are saying to Leave it how it was, but add another extension at the end:

Core -> Default -> BC -> UndocumentedLowPriorityStuff

If I understand this correctly, don't you think that any custom extension added via addExtension() should take priority over some (or most) of these pre-added extensions? Right now addExtension() will place the custom extension at the end -- so will only be called if it's not handled by a different one.

catscarlet commented 1 month ago

This is a terrible design. Which way is Smarty-php going on?