Skip to content
This repository has been archived by the owner on Jul 24, 2024. It is now read-only.

Implement SASS_PATH #1680

Merged
merged 1 commit into from
Aug 26, 2016
Merged

Implement SASS_PATH #1680

merged 1 commit into from
Aug 26, 2016

Conversation

nottrobin
Copy link
Contributor

@nottrobin nottrobin commented Aug 25, 2016

This is intended to implement the SASS_PATH environment variable.

Fixes #1678.

Tests

I've added two new API tests under .render:

  • "should check SASS_PATH in the specified order"
  • "should prefer include path over SASS_PATH"

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

$ fixdir=`pwd`/test/fixtures/sass-path
$ export SASS_PATH=$fixdir/red
$ bin/node-sass $fixdir/index.scss
body {
  background: red; }

Earlier paths are preferred

$ export SASS_PATH=$fixdir/orange:$fixdir/red
$ bin/node-sass $fixdir/index.scss
body {
  background: orange; }

Specified include-paths still take precedence

$ bin/node-sass $fixdir/index.scss --include-path $fixdir/red
body {
  background: red; }

@@ -265,6 +264,18 @@ function normalizeFunctionSignature(signature, callback) {
*/

module.exports.render = function(opts, cb) {
// Add include paths from SASS_PATH environment variable, if present
Copy link
Contributor

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

Copy link
Contributor Author

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.

@nottrobin nottrobin force-pushed the implement-sass-path branch 2 times, most recently from 441ea55 to 640060a Compare August 25, 2016 16:16
@nottrobin
Copy link
Contributor Author

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?

@saper
Copy link
Member

saper commented Aug 25, 2016

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

@nottrobin
Copy link
Contributor Author

Thanks @saper, I hadn't seen that. It's all passed now. @nschonni are you happy that I've address all your points?

@saper
Copy link
Member

saper commented Aug 26, 2016

Looks good to me. Would that be possible to squeeze this into one commit?

@nottrobin
Copy link
Contributor Author

@saper sure no problem

@xzyfer
Copy link
Contributor

xzyfer commented Aug 26, 2016

Nice work everyone. Added this to the next.minor milestone. I believe we have one other PR to land in 3.9.0 also.

@nottrobin
Copy link
Contributor Author

nottrobin commented Aug 26, 2016

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"
@saper saper merged commit 98c05d4 into sass:master Aug 26, 2016
@saper
Copy link
Member

saper commented Aug 26, 2016

Big thank you for the contribution!

@nottrobin nottrobin deleted the implement-sass-path branch August 26, 2016 13:02
@nottrobin
Copy link
Contributor Author

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?

jiongle1 pushed a commit to scantist-ossops-m2/node-sass that referenced this pull request Apr 7, 2024
Unquoting null should result in a deprecation warning
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Is there any plan to support $SASS_PATH?
4 participants