fix rev.js #6

wants to merge 3 commits into


None yet

2 participants



Fixed images match

More powerful usemin for scripts.

mklabs commented Feb 24, 2012

Hi @krzychukula

All of this sounds great.

I see that you removed the necessity of inlined <!--scripts concatenated and end scripts--> comment to perform the replacement for every script tag. I like this change but it may come with some caveats.

Anyway, I'll review, test and merge it asap and get back to you. Thanks!

This was referenced Feb 25, 2012
mklabs commented Feb 25, 2012

I'm really glad you did this pull request.

I've merged it into the dev branch and applied some minor tweaks. Your changes solve quite a few problems with this task that I wasn't able to solve on the first implementation. I did pretty much the same changes for styles and images. It's far better now, thanks again!

If you can test it again from the dev branch, that'd be great. Feel free to re-open this issue if you got any problem with the changes I made.

@mklabs mklabs closed this Feb 25, 2012

Unfortunately I don't have enough permissions to do so.
I found an error.
Doesn't work for:

<script>window.html5 || document.write('<script src="js/libs/html5.js"><\/script>')</script>

In dev version all JS files are reved, even this in libs folder, but this regexp has problem with them.

1: Move my code to original one. I mean first select html fragment within comments, and after that search for scrips files within it. That should be rather easy.
2: Include it in regexp. (attempt to fix regexp)
3: Do not touch libs folder ;)

Do you know how ant-build-script do this?

@mklabs mklabs reopened this Feb 25, 2012
mklabs commented Feb 25, 2012

Okay, reopening :p

I'll take a second try tomorrow.


Problem is about one / in regexp to get prefix and file name.
But this is not always true.

(.+)/(.+)/ make it work, but disables the others...

I think that should be done more universally, but I'm really not into RegExp's so I must do some googling and experiment about it.


My solution in another pull request. Let me know what You think.

@mklabs mklabs closed this Mar 1, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment