canton7 / fuelphp-casset

Better asset management library for fuelphp (with minification!)
MIT License
103 stars 29 forks source link

Twig extension support #18

Closed dmyers closed 12 years ago

dmyers commented 12 years ago

Would you consider adding a twig extension for those who want to use Twig with Casset? I wrote my own, but would rather have it in core and share with others.

     /**
     * Sets up all of the functions this extension makes available.
     *
     * @return  array
     */
    public function getFunctions()
    {
        return array(
            'display_css'      => new Twig_Function_Function('Casset::render_css'),
            'display_js'       => new Twig_Function_Function('Casset::render_js'),
            'display_img'      => new Twig_Function_Function('Casset::img'),
        );
    }

Then in your Parser view config there is an array you set which could be listed on the Casset docs on how to setup.

    'View_Twig' => array(
        'extensions' => array(
            'Twig_Fuel_Extension',
            'Twig_Casset_Extension',
        ),
    ),
canton7 commented 12 years ago

I've never used Twig before, but is it enough to just implement getFunctions? The docs imply that you should add getName and a bunch of other stuff.

In principle, I try and keep Casset as everything-agnostic as possible (although it is very tightly coupled to Fuel). So far, integration with other tools has been done using callbacks, etc. That's not really possible here, though...

I'm just wondering whether a better approach might be to create a dedicated Twig extension which inherits from Twig_Extension and wraps the relevant Casset functions. That way, if someone wants some other tool to be supported, which also requires a getFunctions method, we won't have a conflict.

Such an extension could be enabled simply by uncommenting a line in bootstrap.php. I've been meaning to create something like a contrib/ or extensions/ folder for a while, and this might be a good place to start.

What do you think?

Ninja Edit: just re-read your comment, and realised I'd got the wrong end of the stick completely, and that we were agreeing all along.

Yes, I'd be more than happy to add a Twig extension, so long as it's not enabled/loaded by default.

canton7 commented 12 years ago

This was discussed further in #19, and included in v1.15