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

Allow built-in modifiers override #1049

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

2vcreation
Copy link

A proposition to fix #1048 (and may be a part of #1011 too)

This is to prevent modifiers added by registerPlugin function from beeing silently ignored by DefaultExtension.
More details in #1048

@wisskid
Copy link
Contributor

wisskid commented Jul 31, 2024

Sorry, but I don't like this approach. The registerplugin methods are the "old" way to register plugins. I'm in favor of creating new Extensions.

Wouldn't it be easier to switch the default loading order of the Extensions? It is now: core, default, BC. We could change it to core, BC, default.

@2vcreation
Copy link
Author

Thanks for your quick reply.

You're right, it would have been be easier to switch the default loading order of the Extensions : Core, BC, then Default.
But it does not handle the case where we want to register a "Smarty::PLUGIN_MODIFIER" (not a compiler one), wich has an equivalent in DefaultExtension->getModifierCompiler(). For example :

//This modifier will be silently ignored, even if DefaultExtension is set at the end.
$smarty->registerPlugin(Smarty::PLUGIN_MODIFIER,"json_encode",[$this,"myJsonEncodeModifier"])

In that case DefaultExtension->getModifierCompiler() is going to take the lead anyway.

Side question :
I understand you consider "registerPlugin" method as old way to register Plugins.
Does this mean that it could be deprecated in a near future ?

@wisskid
Copy link
Contributor

wisskid commented Nov 21, 2024

I understand you consider "registerPlugin" method as old way to register Plugins. Does this mean that it could be deprecated in a near future ?

Yes, it might.

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

Successfully merging this pull request may close these issues.

Unable to override built-in modifiers.
2 participants