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.

Sign up for GitHub

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

Jump to bottom

[QUESTION] Some small features - are they needed to PR? #47

Closed
vladimmi opened this issue Mar 12, 2015 · 3 comments
Closed

[QUESTION] Some small features - are they needed to PR? #47

vladimmi opened this issue Mar 12, 2015 · 3 comments

Comments

@vladimmi
Copy link
Contributor

Sorry if this is incorrect place to ask - don't know where to place this question :)
I've forked your minifier and added some small features for my project. They can be a bit unrelated to main goal and not so useful for others so don't know are they worth to create tests and pull request.

  1. ability to generate gzipped file near minified if path specified. So, if you call
$minifier->minify('compressed.css');

you will have two files - compressed.css (as usual) and compressed.gz.css (with gzipped content). Of course, there is setter to disable this.

  1. ability to "inline" external content into JS variables. Something similar to inlining images into CSS. My scripts use HTML files as templates for generated content. To eliminate HTTP requests you can write something like
var myTemplate = '@@import path/to/templates/file.html';

with relative path to template. Minifier will get it, slightly compress (to one line without tabs and double spaces), escape quotes (checking surround ones if they are ' or ") and insert back. So you will have something like

var myTemplate='<html><head>...</head><body>...</body></html>';

in resulting code and can load all scripts and templates with one minified and compressed file.
Import directive ("@@import") can be customized to any string.
@matthiasmullie
Copy link
Owner

Hey @vladimmi

I've been thinking about it.
I definitely like the gzip idea, although I wouldn't do it as part of minify() function. How about creating a new method in Minify.php? Something along these lines:

public function gzip($path, $level = 9)
{
    $content = $this->execute($path);
    $content = gzencode($content, $level)

    // save to path
    if ($path !== null) {
        $this->save($content, $path);
    }

    return $content;
}

What do you think about this?

I also like the other idea, I'm just not really sure it belongs in minifier :p I'll think about that some more.

@vladimmi
Copy link
Contributor Author

How about creating a new method in Minify.php?

Sure, you are free to do it as you wish 👍

I also like the other idea, I'm just not really sure it belongs in minifier :p

That's why I decided to ask before starting to make tests and PR :) Pretty similar to image embedding (make one bigger file instead of many small requests) but... Got some troubles while thinking about that idea:

  1. unlike CSS, such JS code doesn't work without minifier. So it becomes something like builder, not just minifier. Wasn't problem for my project but can be for others.
  2. that can be workarounded but needs some additional JS code. Like var template = Foo.load('path/to/file'); and use that call as directive to embed template. But used paths can be relative to script (which are impossible to resolve from browser, as I know) or relative to root (and we need to set some root path to find them in minifier, making usage harder)...

Maybe you'll have some inspiration and better ideas :)

@matthiasmullie
Copy link
Owner

Just included gzip() method in 30a4592 - thanks for the idea!

I'm not going to go ahead with the other idea, for exactly the same reasons you already listed - haven't been able to come up with great solutions myself :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@matthiasmullie @vladimmi