canton7 / fuelphp-casset

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

Feature/my set group option fix #10

Closed leeovery closed 12 years ago

leeovery commented 12 years ago

Hey,

I tested it and all was working until I added a group via the config file. The options weren't pulling through as you had removed the lines needed to do this (inside the foreach loop in _init [~ line 165]). Ive added in a method for prepping the group options. Just an idea but doing it this way means you dont have to update so much code when adding new options to the defaults. The prep method uses the default array to loop through and check the group options.

Ive tested with groups from config and inline (from controller). Ive put the following lines above the inline new group call (Casset::add_group) and this works as expected.

Casset::set_jsoption('', 'min', false); Casset::set_jsoption('', 'combine', false);

If a new group has options specified those are used rather than updated defaults (as expected).

The above lines work perfectly with all assets I tested. Cant see anything else wrong.

Commit Msg: Added option prep method so group options are passed through to the add_group method. Defaults are merged in via add_group_base. Groups which were defined via the casset/config.php were not being passed through following previous commits.

I also wondered whether it would be easier to contain the group options (from the config file) inside an array with the key == options. You could just do a simple merge then. Obviously causes issues with backwards compatibility though...

    'groups' => array(
        'js' => array(
            'jquery' => array(
                'files' => array(
                    array('google_api::jquery/1.6.2/jquery.js', 'google_api::jquery/1.6.2/jquery.min.js'),
                ),
                'options' = array(
                    'enabled' => true,
                    'combine' => false,
                ),
            ),
        ),
    ),

Lee

canton7 commented 12 years ago

Yeah it's that backwards compat I'm worried about. When that decision was made, there were virtually no options (maybe even just 'enabled').

At some point, I'd like to make Casset more lazy, and only set the group options that differ from the defaults. But that's for another day...