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

ctx->includedFiles misleading stdin #634

Closed
am11 opened this issue Nov 9, 2014 · 6 comments · Fixed by sass/node-sass#543
Closed

ctx->includedFiles misleading stdin #634

am11 opened this issue Nov 9, 2014 · 6 comments · Fixed by sass/node-sass#543
Assignees

Comments

@am11
Copy link
Contributor

am11 commented Nov 9, 2014

Consider the following structure:

c:/temp/alpha/index.scss

@import 'foo.scss';
@import 'bar.scss';

c:/temp/alpha/foo.scss

body {
  color: red;
}

c:/temp/alpha/bar.scss

p {
  color: grey;
}

With node-sass, if you do:

//# while you are in node-sass directory, enter node-interactive-console by typing
$ node
>
var stats = {};
require("./lib/").render({
   data: fs.readFileSync('c:/temp/alpha/index.scss','utf8'),
   stats: stats,
   includePaths: ['c:/temp/alpha']
});

console.log(stats);

It gives:

> { entry: 'data',
  start: 1415551465290,
  includedFiles: [ 'c:/temp/alpha/foo.scss', 'stdin' ],
  end: 1415551465325,
  duration: 35,
  sourceMap: undefined 

in stats. The expected behavior is to have includedFiles: ['c:/temp/alpha/foo.scss', 'c:/temp/alpha/bar.scss'] instead of stdin. Since this is data input, otherwise it would also contain reference to index.scss, had it be the file input (which it produces correctly with require("./lib/").renderSync({file: 'c:/temp/alpha/index.scss', ..).

/cc @mgreter (I tested with your branch: api/c-interface, but still the same issue)

@mgreter
Copy link
Contributor

mgreter commented Nov 9, 2014

ACK, it probably shouldn't be on that array. Sorry, I was not really passing included_files back till lately in perl-libsass, therefore I haven't had any unit tests for this! Unfortunately the array gets sorted before returning, therefore you'll get stdin on that position. Since it is only a status array and not used internally it should be trivial to add a check before adding it to the array (also because the other paths are absolute, which means "stdin" is not a valid path for any other file).

On another issue, I have removed num_included_files in favor of using a null terminated "array". But sass_interface is still using it. I probably just have to copy the updated code from the new sass_context to fix this!

@am11
Copy link
Contributor Author

am11 commented Nov 9, 2014

Thanks @mgreter! 👍

So basically, from the above example, bar.scss is there in the returned array but num_included_files is returning wrong count? This only happens in case of data input.

@mgreter
Copy link
Contributor

mgreter commented Nov 9, 2014

Was a bit more complex! Aded some more info to this commit message!

@am11
Copy link
Contributor Author

am11 commented Nov 9, 2014

Awesome! 😄 👍

@mgreter
Copy link
Contributor

mgreter commented Nov 17, 2014

This should be fixed by 9b984fa

@mgreter mgreter closed this as completed Nov 17, 2014
@am11
Copy link
Contributor Author

am11 commented Nov 17, 2014

Indeed this is fixed. 😃 👍

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 a pull request may close this issue.

2 participants