DigitPaint / roger

Roger is your friendly front-end development toolbox!
MIT License
4 stars 4 forks source link

release.getfiles also returns directories #35

Closed dannycobussen closed 9 years ago

dannycobussen commented 9 years ago

In the Roger::Release::Injector class release.getfiles(@options[:into]) is called, see https://github.com/DigitPaint/roger/blob/master/lib/roger/release/injector.rb#L49. I've a mockup and In the mockupfile the injector is called to inject a banner into the pattern **/*.js,**/*.css.

The thing is in a certain mockup I've a JS library file placed at '/vendor/raf.js/raf.min.js', yes that's correct a folder named 'raf.js'. The injector tries to retrieve all the files with this pattern, by calling release.getfiles. This getfiles function uses the glob to get all files and it also founds the folder 'raf.js', because it's not checking/validating wether it's a file. (https://github.com/DigitPaint/roger/blob/master/lib/roger/release.rb#L206). So its missing a check to validate it collects only files at the function 'getfiles'. :)

Another solution could be to exclude the 'vendor' folder for the injector, but the injector doesn't support a exclude/skip array.

edwinvdgraaf commented 9 years ago

Not sure. But I think you can supply a negation in a glob something like !(vendor). Worth trying. Not neglecting your proposed change tho. The method clearly doesn't align its name with its behaviour. But may keep you going for the moment.

Sent from my iPhone

On 23 jun. 2015, at 14:06, Danny Cobussen notifications@github.com wrote:

In the Roger::Release::Injector class release.getfiles(@options[:into]) is called, see https://github.com/DigitPaint/roger/blob/master/lib/roger/release/injector.rb#L49. I've a mockup and In the mockupfile the injector is called to inject a banner into the pattern */.js,*/.css.

The thing is in a certain mockup I've a JS library file placed at '/vendor/raf.js/raf.min.js', yes that's correct a folder named 'raf.js'. The injector tries to retrieve all the files with this pattern, by calling release.getfiles. This getfiles function uses the glob to get all files and it also founds the folder 'raf.js', because it's not checking/validating wether it's a file. (https://github.com/DigitPaint/roger/blob/master/lib/roger/release.rb#L206). So its missing a check to validate it collects only files at the function 'getfiles'. :)

Another solution could be to exclude the 'vendor' folder for the injector, but the injector doesn't support a exclude/skip array.

— Reply to this email directly or view it on GitHub.

flurin commented 9 years ago

Get files should only return files, see PR #36

flurin commented 9 years ago

Fixed in #36