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

CssMin #13

Open
Swader opened this issue Aug 2, 2015 · 11 comments
Open

CssMin #13

Swader opened this issue Aug 2, 2015 · 11 comments

Comments

@Swader
Copy link
Contributor

Swader commented Aug 2, 2015

I'm having problems using SimpleCssMin in that it doesn't resolve relative paths. For example, compiling FontAwesome (trying to get this installed and compiled via BowerPHP), I get this:

screenshot 2015-08-03 01 37 37

I then tried to use CssMin, as per package suggestion post-install, but I get this:

Fatal error: Call to undefined method CssMin::settings() in /home/vagrant/.composer/vendor/markstory/mini-asset/src/Factory.php on line 202

Is there something I'm doing wrong?

@Swader
Copy link
Contributor Author

Swader commented Aug 3, 2015

I should note that copying fonts from bower_components to public/css makes things work in the first case, but I'd rather either:

  1. Have the option to direct copy a folder to cachePath with mini-asset, like so:

    [sync]
    from = bower_components
    to = public/css
    files[] = ...
    folders[] = font-awesome/fonts
    

    Several sync blocks could be defined

  2. mini-asset detects relative paths and adjusts them accordingly.

@markstory markstory added this to the 1.0.2 milestone Aug 3, 2015
@markstory
Copy link
Owner

What does your filter block look like? It should contain CssMinFilter not CssMin.

Fixing relative paths is hard as mini-asset doesn't know where the webroot is relative to the source files. If relative path resolution was to be added this would need to be added as well.

Adding file syncing has come up once before, but there was never a nice implementation/proposal for how it should work. In the applications I have, I just don't use relative paths.

@Swader
Copy link
Contributor Author

Swader commented Aug 3, 2015

Ah, all good when I put CssMinFilter, thanks.

Regarding rel paths, I understand, it's a tricky one, yes.

In the applications I have, I just don't use relative paths.

Not really a solution though, is it? Some outside packages will use them, and that's where we have a problem.

How do you feel about my approach above, for syncing? Define several sync blocks, each can have its from and to option and takes an array of folders and/or files.

@markstory
Copy link
Owner

I didn't intend to imply that relative paths are bad, and should never be used. I was trying to give background on why relative path resolution hasn't been implemented so far. I can see how relative paths would be useful though.

We might need to use sync as a prefix. As PHP array's (which the ini is converted into) won't allow duplicate keys.

Something like:

; from path is in section name
[sync-bower_components/autocomplete]
; destination path.
to = public/css
; glob pattern for matching files inside bower_components/autocomplete
matching = *.img

The above might be a terrible idea though.

@Swader
Copy link
Contributor Author

Swader commented Aug 4, 2015

It doesn't look that healthy, but could work. Maybe better with a colon though:

[sync:bower_components/autocomplete]

Have you considered moving off of ini and into something more versatile like simple PHP arrays, or plan old JSON?

@markstory
Copy link
Owner

JSON is horrible for config files as there is no way to include comments. Basic PHP arrays could work well though.

The colon syntax looks fine to me, better than what I had proposed.

@Swader
Copy link
Contributor Author

Swader commented Aug 4, 2015

JSON is horrible for config files as there is no way to include comments.

True. How about Neon?

@markstory
Copy link
Owner

toml is another good option 🚲

@Swader
Copy link
Contributor Author

Swader commented Aug 4, 2015

Actually, all this just makes mini-asset heavier and more complicated than necessary. I suggest staying with INI but with sync: prefixes, or going full PHP. I'll see if I can whip up a PR or two this week, if time allows.

@markstory
Copy link
Owner

Requiring external libraries is another reason I initially went with ini files. Its built-in and doesn't have any of the drawbacks that JSON does, and cannot crash an application due to a parse error like PHP config files can.

@biesbjerg
Copy link

I hacked together a filter for AssetCompress (I'm using and old modified version, that passes file content as well as the file path to the filter) that rewrites relative paths into root absolute paths. It could probably use some refactoring, but might give you some ideas:

<?php
App::uses('AssetFilterInterface', 'AssetCompress.Model');
class CssRewriteUrlFilter implements AssetFilterInterface {

    protected $_pattern = '/url\s*\(\s*[\'"]?\s*([\w.:\/\-_#?&=]+)\s*[\'"]?\s*\)/ims';

    public function filter($content, $path) {
        return preg_replace_callback($this->_pattern, function($matches) use ($path) {
            $url = $matches[1];

            // Leave absolute urls alone
            if ($this->_isAbsoluteUrl($url)) {
                return $matches[0];
            }

            // Separate url from query string often used for cache busting
            if (strpos($url, '?') !== false) {
                list($url, $queryString) = explode('?', $url);
            }

            // System path to url currently being processed
            $systemPath = dirname($path) . '/' . $url;
            if (file_exists($systemPath)) {
                $systemPath = realpath($systemPath); // Normalize path
            }

            // Rewritten url
            $newUrl = '/' . str_replace(WWW_ROOT, '', $systemPath);

            // Add back query string
            if (!empty($queryString)) {
                $newUrl = $newUrl . '?' . $queryString;
            }

            return sprintf('url(\'%s\')', $newUrl);
        }, $content);
    }

    protected function _isAbsoluteUrl($url) {
        return preg_match('|^\w+://|', $url);
    }

}

@markstory markstory modified the milestones: 1.0.2, 1.0.3 Jan 17, 2016
@markstory markstory modified the milestone: 1.0.3 Jan 30, 2016
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

No branches or pull requests

3 participants