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

Do not minify already minified files #75

Merged
merged 7 commits into from
Oct 1, 2015
Merged

Do not minify already minified files #75

merged 7 commits into from
Oct 1, 2015

Conversation

DarkaOnLine
Copy link
Contributor

Some times JSMin will throw "Unterminated set in RegExp at byte" exception if the file is already minified. See more: http://www.mrclay.org/2013/11/27/jsmins-classic-delimma-division-or-regexp-literal/

Added possibility to exclude extensions from minification process.

@DarkaOnLine
Copy link
Contributor Author

@Stolz Any comments on this ?

@Stolz
Copy link
Owner

Stolz commented Sep 2, 2015

@DarkaOnLine Sorry for the delay, I'm currently on vacation. I'll review it a few days. Thanks for your contribution.

@DarkaOnLine
Copy link
Contributor Author

@Stolz ?

@Stolz
Copy link
Owner

Stolz commented Sep 30, 2015

@DarkaOnLine I'm not sure about this implementation, particularly with the use of loops to compare strings. Do you think you could use a regex approach similar to the one used for the '*_regex' config options?.

If you take a look the current '*_regex' options do not use the file extension to know what to include/exclude. That way final users can have exclude a wider range of options.

@DarkaOnLine
Copy link
Contributor Author

Are talking about stringContains method ?

@Stolz
Copy link
Owner

Stolz commented Sep 30, 2015

Yes. I think using preg_match() could do the trick

@DarkaOnLine
Copy link
Contributor Author

OK. will change it

@DarkaOnLine
Copy link
Contributor Author

Changed comparison to regexp

*
* @var array
*/
//'exclude_minification_regex' => '/(.min.js|-min.js)$/i';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(pedant)

. is a wildcard, which matches -.

Perhaps [-.]min\.js$

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right. Yours pattern even significantly faster:

Time 1: 4.6014785766602E-5
Time 2: 1.8835067749023E-5

Stolz added a commit that referenced this pull request Oct 1, 2015
Closes #75
Merge remote-tracking branch 'pullrequest/exclude_minification' into develop
@Stolz Stolz merged commit 1427d5c into Stolz:master Oct 1, 2015
@Stolz
Copy link
Owner

Stolz commented Oct 1, 2015

This has been merged. I refactored it a bit and the option has been renamed to no_minification_regex.

Thanks again for your contribution.

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

Successfully merging this pull request may close these issues.

None yet

3 participants