Extension-based filters not working as expected about assetmanager HOT 8 CLOSED

@glen-84 AFAIK, filtering applies to single assets (or asset collections) - not automagically applied

from assetmanager.

@Ocramius Nooooo, I don't wanna have to list every single file. :-(

Assetic supports glob assets which are collections ... does AssetManager support this?

Edit: Actually that would probably merge them, which I don't want.

from assetmanager.

@grizzm0 can you provide feedback here? Seems like you have a solution

from assetmanager.

Sorry, took this discussion on IRC instead. Simply replace 'less' with 'css' in the filters array. It's based on mime type and not extension from the looks of it. And less has the mime of text/css.

from assetmanager.

Right, this works:

        'css' => array(
            array('filter' => 'Lessphp')
        )

TBH, I don't understand how AM distinguishes between a mime type and an extension. For example:

    'less' => array(
        array('filter' => 'Lessphp')
    )

Is 'less' a mime type or an extension here? I intended for it to be an extension, but it seems that this doesn't work.

Perhaps it should be more like:

'extensions' => array(
    'less' => array(
        array('filter' => 'Lessphp')
    )
)

from assetmanager.

The problem is that the extension is determined base on the mime type of the file, not the actual extension.

Have a look at https://github.com/RWOverdijk/AssetManager/blob/master/src/AssetManager/Service/AssetFilterManager.php#L75-L82 and https://github.com/RWOverdijk/AssetManager/blob/master/src/AssetManager/Service/MimeResolver.php#L577-L584.

Let's say I have this configuration:

'filters' => array(
    'less' => array(
        array('filter' => 'Lessphp'),
    ),
),

and I'm trying to load the file /path/to/hello-world.less. Since I have neither the path nor the mimetype in the configuration, AssetFilterManager is going to go into the logic block for extension, shown below:

$extension = $this->getMimeResolver()->getExtension($asset->mimetype);
if (!empty($config[$extension])) {
    $filters = $config[$extension];
} else {
    return;
}

So, basically, if the extension comes back as less and we have a less key in the filters config. The problem is that the extension is coming back from the MimeResolver as css, and here's why:

if (!($extension = array_search($mimetype, $this->mainMimeTypes))) {
    $extension = array_search($mimetype, $this->mimeTypes);
}

return !$extension ? null : $extension;

Have a look at MimeResolver::$mimeTypes - this array contains several instances of the text/css mime type, shown below:

    'css'      => 'text/css',
    'less'     => 'text/css',
    'sass'     => 'text/css',
    'scss'     => 'text/css',

The issue is that array_search(...) is going to return the key of the first found instance of what's being searched for. We're looking for the text/css mimetype, and the first instance is css, so MimeResolver reports back that the file's extension is css, which is incorrect.

I believe the best solution here is to use php's pathinfo(...) function to determine the extension.

Remove: $extension = $this->getMimeResolver()->getExtension($asset->mimetype);
Replace with: $extension = pathinfo($asset->getSourcePath(), PATHINFO_EXTENSION);

This will give us the file's actual extension regardless of the mimetype.

I would happily send a PR for this, but I can't get the test suite to run and there are no instructions on how to do so. :)

from assetmanager.

The instructions are in the travis file :)

We could match against both extensions, as to not break BC, but I'm not sure if we really want that. If you do so, the files will be stored as less, too. That will cause problems for most people.

from assetmanager.

The instructions are in the travis file :)

Oh yeah! Oops... I will blame my failure to notice that on not having had enough coffee yesterday. :)

I'm not sure what you mean about the files being stored as less too, though. If you mean that *.css files will be passed through the Lessphp filter when they previously were not, then that shouldn't happen. That would only happen if css is specified as the extension in the filter config, which means that's what the user wanted anyway (and that presently works today).

The pathinfo(...) suggestion I think would give full BC. I can put together a PR when I have some time see what you think.

from assetmanager.