noiselabs / SmartyBundle

Smarty3 template engine bundle for Symfony
http://smartybundle.readthedocs.io/
GNU Lesser General Public License v3.0
51 stars 36 forks source link

improve "plugin already registered" verification #52

Closed naucon closed 8 years ago

naucon commented 8 years ago

As you suggested earlier:

Improve "plugin already registered" verification by accessing the smarty plugin registry directly instead of keeping a second state in the bundle.

Would be preferable to "ask" Smarty if the plugin is already registered to avoid keeping state in two places leading to inconsistency potentially. Not sure if Smarty provides this though.

naucon commented 8 years ago

In our project we use the form-extension branch. could you merge both pull request to this branch too.

vitorbrandao commented 8 years ago

Good find @naucon :+1:

About the form-extension it isn't considered stable. I would like to catch-up with @belka-ew since he was working on it. Have you done any changes internally to that branch?

naucon commented 8 years ago

Thanks for the merge.

Is use the form-extension branch only for the csrf support. I haven't done any changes to it.

belka-ew commented 8 years ago

form-extension was usable for some time but didn't work after some Symfony or Smarty updates. Maybe I fixed something in my fork but I deleted it and unfortunately I left the PHP development almost completely some time ago.

naucon commented 8 years ago

@noisebleed: i will switch to master branch and add the CSRF-Support through a plugin or extension (need it for login). I can contribute the csrf plugin to the core if your interessed.

vitorbrandao commented 8 years ago

OH. Anyway, thanks for the feedback and all the effort you have put on the project. Much appreciated, @belka-ew .

vitorbrandao commented 8 years ago

@naucon, is this a Symfony/Twig helper? Or something you built yourself?

Btw, started https://github.com/noiselabs/symfony-smarty-edition to use as a sandbox for testing SmartyBundle features and the symfony-3.x support. Haven't pushed my latest changes but it will provide a Docker container preloaded with Symfony+SmartyBundle.