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

sass_compiler_get_last_import() does not provide previous path #1035

Closed
rodneyrehm opened this issue Apr 2, 2015 · 9 comments · Fixed by #1038
Closed

sass_compiler_get_last_import() does not provide previous path #1035

rodneyrehm opened this issue Apr 2, 2015 · 9 comments · Fixed by #1038

Comments

@rodneyrehm
Copy link
Contributor

I'm trying to upgrade to 3.2.0-beta.4 following the notes in #1000.

The old way:

struct Sass_Import** sass_importer(const char* url, const char* prev, void* cookie) {
  return do_something_fun_with(url, prev);
}

The new way:

struct Sass_Import** sass_importer(const char* cur_path, Sass_Importer_Entry cb, struct Sass_Compiler* comp) {
  struct Sass_Import* previous = sass_compiler_get_last_import(comp);
  const char* prev_path = sass_import_get_path(previous);
  return do_something_fun_with(cur_path, prev_path);
}

The problem here is that prev_path always is an empty string (and sass_import_get_base(previous) is nullptr). I'm running a with Sass_Data_Context if that makes any difference. Having the sass_importer return NULL will load the proper file. What am I missing?

@drewwells
Copy link
Contributor

The prev_path returns the parent which contains the import currently being requested at cur_path.

If I'm understanding you correctly, you are using the data compiler. When using the data compiler, the prev_path is set to stdin as there is no file path it can be connected to. I've never gotten empty string for this property. If that file goes onto list another import, prev_path will be the current file.

Here's code thats used with the file and data compiler, if that helps: https://github.com/wellington/wellington/blob/master/context/importer.go#L9

data compiler
prev_path == stdin cur_path == import

file compiler
prev_path == file cur_path == import

@rodneyrehm
Copy link
Contributor Author

my importer function currently doesn't do more than log its input and return NULL. I have the following FS:

/sass/testfile.scss
  @import "sub/deeptest";\n.testfile { content: "loaded"; }
/sass/sub/deeptest.scss
  .deeptest { content: "loaded"; }

The source I pass to the Sass_Data_Context is as follows:

@import "testfile";

The generated content is correct:

.deeptest{content:"loaded"}.testfile{content:"loaded"}

My importer looks as follows:

struct Sass_Import** sass_importer_emscripten(const char* cur_path, Sass_Importer_Entry cb, struct Sass_Compiler* comp) {
  struct Sass_Import* previous = sass_compiler_get_last_import(comp);
  const char* prev_path = sass_import_get_path(previous);
  const char* prev_base = sass_import_get_base(previous);

  consoleLog(strdup("path"), strdup(cur_path));
  consoleLog(strdup("prev.path"), strdup(prev_path ? prev_path : "-nullprt-"));
  consoleLog(strdup("prev.base"), strdup(prev_base ? prev_base : "-nullprt-"));

  return NULL;
}

The expected log output:

path: "testfile"
prev.path: "stdin"
prev.base: -nullptr-

path: "sub/deeptest"
prev.path: "/sass/testfile.scss"
prev.base: "stdin"

The actual result:

path: "testfile"
prev.path: ""
prev.base: -nullptr-

path: "sub/deeptest"
prev.path: ""
prev.base: -nullptr-

@rodneyrehm
Copy link
Contributor Author

for what its worth, I'm getting the same output when using Sass_File_Context

@drewwells
Copy link
Contributor

Thanks for adding your input, that helps me figure out a lot!

Which compiler are you using? I assume when you say you're switching Context objects, you're also using the appropriate compiler. I modified my importer to do the same prints as yours.

path: testfile
prev.path: stdin
prev.base: stdin

path: sub/deeptest
prev.path: testfile
prev.base: testfile.scss

As you can see I never get empty strings or null pointers, always file path or stdin. Which commit hash of libsass are you on? I'm using d215db5

@drewwells
Copy link
Contributor

There is an issue with sass_import_get_base when used with the data compiler. The file compiler reports the absolute path to the import, the data compiler is reporting only the filename relative to working directory.

@rodneyrehm
Copy link
Contributor Author

Which commit hash of libsass are you on?

I checked with 3.2.0-beta.4 and fd7f20fa (most recent master at the time of writing).

Which compiler are you using?

I don't understand the question. Is there more than one? I'm using the C-API and after configuring the options I call sass_compile_data_context(). The time I confirmed my results with the file context, I used sass_compile_file_context(), obviously. Does that count as two compilers?

The way I use libsass (without the modifications for the beta.4 importer) can be observed here. Should I be doing something different?

@mgreter
Copy link
Contributor

mgreter commented Apr 2, 2015

OK, I can confirm that this is a bug if you use sass_compile_xxx_context. It works correctly if you use sass_make_xxx_compiler. I will be able to create a PR to fix this. Sorry for the inconvenience!
@am11 this is the same bug you mentioned in sass/node-sass#827 (comment)?

@mgreter mgreter added this to the 3.2 milestone Apr 2, 2015
@mgreter mgreter self-assigned this Apr 2, 2015
mgreter added a commit to mgreter/libsass that referenced this issue Apr 2, 2015
mgreter added a commit to mgreter/libsass that referenced this issue Apr 2, 2015
@drewwells
Copy link
Contributor

Ahh! This makes more sense now.

@am11
Copy link
Contributor

am11 commented Apr 3, 2015

Yay! for the issue being spotted! 😎 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants