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

Consistent file handling #66

Closed
wants to merge 2 commits into from

Conversation

idflood
Copy link

@idflood idflood commented Jul 1, 2014

Currently the file handling is different depending of the sourceComments option.

  • when sourceComments is map or normal it use file.path as parameter
  • on all other cases it set opts.data to be the stringified file.

This make the .sass indented files to not compile with the default parameters. node-sass or libsass need to know the extension to properly compile sass file which is not the case with the stringified content.

With the first commit only there was 2 tests failing. I think it was caused by the fact that these 2 tests where the only one with no real files (the error was that the file was not found).

Let me know if I can make anything to make this pull request happen.

@raineorshine
Copy link

Anything I can do to help this along? Took me a while to trace through the github issues and changelogs to make sense of the situation. Awesome that it's here now, and the sooner this gets into master without the sourceComments hack, the fewer people will have to go through the trouble I did to see that it actually is supported.

Thanks @idflood for the PR!

@jmm
Copy link

jmm commented Jan 2, 2015

These changes would exacerbate an existing problem with gulp-sass. See gulp-sass discards changes from earlier in the pipeline when generating source maps #158. (And even if that weren't the case, it would be ignoring the file content that gulp has already read into a buffer only for node-sass to read it from disk again). I'm not sure how to solve the .sass problem, but this isn't the way.

@Keats
Copy link
Collaborator

Keats commented Mar 25, 2015

gulp-sass v2 will just pass the files to node-sass so this won't be needed

@Keats Keats closed this Mar 25, 2015
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.

4 participants