-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
@@ -265,6 +264,18 @@ function normalizeFunctionSignature(signature, callback) { | |||
*/ | |||
|
|||
module.exports.render = function(opts, cb) { | |||
// Add include paths from SASS_PATH environment variable, if present |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This normalization probably belongs up in the getOptions options.includePaths = (options.includePaths || []).join(path.delimiter);
, then there is no change anything else
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tidied this up.
441ea55
to
640060a
Compare
How can a travis build take quite so long? @nschonni is something up? Is there some way for me to start the travis build again? |
Looks like OSX jobs got stuck and couldn't even be started. here's an update from Travis CI https://www.traviscistatus.com/incidents/4mvp857qx8bw |
Looks good to me. Would that be possible to squeeze this into one commit? |
@saper sure no problem |
Nice work everyone. Added this to the next.minor milestone. I believe we have one other PR to land in 3.9.0 also. |
640060a
to
3788c5d
Compare
Okay commits squashed into 98c05d4. |
- New function buildIncludePaths to build the includePaths string for passing through to libsass - In buildIncludePaths, check for SASS_PATH environment variable, and if found include them behind any other specified include-paths - Add two tests to `.render(options, callback)`: - "should check SASS_PATH in the specified order" - "should prefer include path over SASS_PATH"
3788c5d
to
98c05d4
Compare
Big thank you for the contribution! |
No problem. Thanks for all your help with this @saper et al., it was fun! Do you know when the next minor release will be? |
Unquoting null should result in a deprecation warning
This is intended to implement the
SASS_PATH
environment variable.Fixes #1678.
Tests
I've added two new API tests under
.render
:If you run
mocha test/api.js
you should see these tests pass.Manual checking
You can test it manually using the test fixtures as follows:
SASS_PATH is picked up
Earlier paths are preferred
Specified include-paths still take precedence