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

PR: Add an option for a banner (#45) #62

Merged
merged 1 commit into from
Aug 21, 2013
Merged

Conversation

drublic
Copy link
Contributor

@drublic drublic commented Aug 20, 2013

A banner is a string prepended to the specified output file.

This Pull Request closes #45.
Inspired by gruntjs/grunt-contrib-compass#18.

A banner is a string prepended to the specified output file.

Closes gruntjs#45.
Closes gruntjs#62.
sindresorhus added a commit that referenced this pull request Aug 21, 2013
PR: Add an option for a banner (#45)
@sindresorhus sindresorhus merged commit e8f2044 into gruntjs:master Aug 21, 2013
@sindresorhus
Copy link
Member

Awesome. Thanks man :D

@drublic drublic deleted the banner branch August 21, 2013 10:46
@aaronbarker
Copy link

Thanks @drublic you are my hero :)

@drublic
Copy link
Contributor Author

drublic commented Aug 21, 2013

Wheee!

@aaronbarker
Copy link

It looks to me that this may be breaking the sourcemaps references. If I don't add the banner option, sourcemap references in Chrome 29 all point to the correct lines in the correct files. If I do include the banner option, they are all pointing to the wrong place (file and line).

I'm just guessing here, but if the banner is added after the sourcemap has made the map, that may cause problems. If the sourcemap logic determines that line 17 of the rendered CSS points to primary.scss line 39, and then you add a 3 line banner, line 17 is now 20 and so doesn't track to the correct scss file and line any more.

Could that be the problem? Can anyone confirm the issue?

@aaronbarker
Copy link

Based on the documentation update to say that the banner option isn't compatible with the sourcemap option, is the above issue something that can't be fixed? Or is that documentation update just a placeholder until it is fixed?

@sindresorhus
Copy link
Member

@aaronbarker it might be possible to fix if there's a module that let's put in a file + sourcemap, modify the file, then get an updated sourcemap. Not that i know of though. So the easiest right now is to just make them mutually exclusive. PR welcome though.

@aaronbarker
Copy link

Poked around and found this gruntjs/grunt-contrib-uglify#22 so we aren't the only one with the problem. Will continue to ponder on it. Thanks.

@drublic
Copy link
Contributor Author

drublic commented Aug 29, 2013

Is there a solution for the grunt-contrib-compass plugin? Maybe we can adapt this?

@sindresorhus
Copy link
Member

Nope

@drublic
Copy link
Contributor Author

drublic commented Aug 29, 2013

Bummer.

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.

Add a banner option
3 participants