Improve API (sourceRoot option and importer errors)

[mgreter]

Addresses #880 (//CC @chriseppstein)

I opted to extend the internal struct to also carry error options.
To return a error from custom importers you need to:

char* message = SvPV_nolen(ERRSV);
incs = sass_make_import_list(1);
incs[0] = sass_make_import_entry(0, 0, 0);
sass_import_set_error(incs[0], message, line, col);

It will then throw an error as with regular imports that fail.

[am11]

👍 💯

[am11]

@mgreter, since the implementer doesn't have any context to resolve line and column information in importer, wouldn't it make sense that libsass handle those two at its own and just accept the error message?

[mgreter]

The implementor could have a context, like testing if the content it is loading conforms to the rules it expects. IMO it could give more details where the actual error occurred, so I'm not really convinced to remove it. But you may be right that it is a bit over engineered. I guess it's no harm in leaving it in for now?

[drewwells]

It works, awesome @mgreter !

So I'm trying to understand your discussion about context. Right now, libsass expects the implementor passes along the error line number/column. Would it be possible to provide error line number of where the @import line is? If that's already being passed, apologies for missing that.

I'm asking if sass_import_set_error is only passed a import list entry and a message (really no message would be fine with me "import not found: {name}" is all I will be putting in there. Here's the change I'm asking about:

    C.sass_import_set_error(ent, errMessage)

[mgreter]

I added a switch, so if you set line and col to -1 (string::npos), it will throw the error on behalf of the parent context. This way you can just set the error message and leave the rest to libsass.

C.sass_import_set_error(ent, errMessage, -1, -1)

[drewwells]

Can size_t be negative? I'm getting an error assigning -1.

constant -1 overflows C.size_t

[mgreter]

Then you probably need to use string::npos, actually it is the biggest number possible, but mostly -1 should overflow to the biggest number (AFAIK) 😉 seems to work in C ...

[am11]

@drewwells,

really no message would be fine with me "import not found: {name}"

I think its not mandatory to return custom importer error for implementer. If you don't need the customized error, you can exclude it and libsass will fallback to its default missing-import error.

[drewwells]

string::npos is C++, I used C.UINTMAX_MAX to get -1 in a size_t (in case anybody else comes across the same situation).

@am11 the error isn't mandatory, but returning any non-empty entry list uses that as the missing import. Returning an empty list or size 1 with nothing in it, results in a runtime exception. So I'm returning an error to override the blank import entry that seems to be required to do this.

Here's the code, if there's a better way to do it: https://github.com/wellington/wellington/blob/importer/context/export.go#L41

[mgreter]

You could return a list with one entry, where you only set the filename (no data). Libsass will then try to load that file through it's loading code, which would emit the error @am11 probably meant. But IMO you should only do that if you are sure the file does not exist, otherwise it could have strange side effects!

[drewwells]

I'm not great at C, what do I pass to sass_make_import_entry() to do that?
I'm currently passing empty char* but that is succeeding as a successful
import of an empty file.

On Sat, Mar 7, 2015 at 12:03 PM Marcel Greter [email protected]
wrote:

You could return a list with one entry, where you only set the filename
(no data). Libsass will then try to load that file through it's standard
loading code, which would emit the error @am11 https://github.com/am11
probably meant. But IMO you should only do that if you are sure the file
does not exist, otherwise it could have strange side effects!

—
Reply to this email directly or view it on GitHub
#915 (comment).

[mgreter]

sass_make_import_entry("non_existing_file", 0, 0);

[drewwells]

Thanks for your patience, it's working great. Sending nil did the expected behavior

error > stdin:1
    file to import not found or unreadable: non_existing_file