rstacruz / sinatra-assetpack

Package your assets transparently in Sinatra.
http://ricostacruz.com/sinatra-assetpack/
MIT License
541 stars 97 forks source link

Filter dyn_local_file_for for the same file extension as the original. #75

Closed mikesten closed 11 years ago

mikesten commented 11 years ago

I've got a /packages directory in /public. In there is /select2, which has select2.js, select2.css and select2.png. I was getting the .png file returned for /packages/select2.005533.js because dyn_local_file_for looks for "#{file}.*", which was fetching all of the above.

I've added an extra line to remove files that don't have the same extension as the original to stop this from happening.

j15e commented 11 years ago

Your pull request build is failing because you cannot mach only the requested extension as select2.js might refer to source select2.coffee and select2.css might refer to select2.sass.

Are you using the very latest version as I am pretty sure I fixed this issue in v0.1.1 regarding #50 and commits 30a1a9f27d60cd120fc9f46f6573004407ce995b and 32dda3a2d793aa9d16d24fa974a46fcd44106d55.

j15e commented 11 years ago

PS run the tests locally using rake command (default rake task is test)

mikesten commented 11 years ago

Ah, I ran the tests but get a load of failures so assumed they were out of date. I'm using 0.1.2 and it's definitely still an issue... I'll go check the tests now.

On 30 January 2013 18:01, Jean-Philippe Doyle notifications@github.comwrote:

Your pull request build is failing because you cannot mach only the requested extension as select2.js might refer to source select2.coffeeand select2.css might refer to select2.sass.

Are you using the very latest version as I am pretty sure I fixed this issue in v0.1.1 regarding #50https://github.com/rstacruz/sinatra-assetpack/issues/50and commits 30a1a9fhttps://github.com/rstacruz/sinatra-assetpack/commit/30a1a9f27d60cd120fc9f46f6573004407ce995band 32dda3ahttps://github.com/rstacruz/sinatra-assetpack/commit/32dda3a2d793aa9d16d24fa974a46fcd44106d55 .

— Reply to this email directly or view it on GitHubhttps://github.com/rstacruz/sinatra-assetpack/pull/75#issuecomment-12903091.

Mike Stenhouse Designer, programmer and maker of things. @mikesten mikesten.com

mikesten commented 11 years ago

That's not entirely true, thinking about it. There was a fix between 0.1.2 and master where matches.sort! { |f| became matches.sort! { |f, _|... I pulled that fix over to our code as a monkey patch (long story) and found that instead of 500ing I was being served a .png instead.

mikesten commented 11 years ago

How about this? Tests pass but is it sane?

j15e commented 11 years ago

Yep seems good, it would be "ready to merge" if you could providing a test for this specific issue & sqwash the 2 commits (or submit a patch with only the second commit).

Thanks

mikesten commented 11 years ago

Okey dokey. Test case added and commits crushed...

j15e commented 11 years ago

Awesome, thanks @mikesten

j15e commented 11 years ago

Released in v0.1.3