support custom import extension #246
Conversation
sass.py
Outdated
| s, v, _ = _sass.compile_filename( | ||
| input_filename, output_style, source_comments, include_paths, | ||
| precision, None, custom_functions, importers, None, | ||
| precision, None, custom_functions, importers, None, [], |
There was a problem hiding this comment.
compile_dirname should probably grow this parameter as well
sass.py
Outdated
|
|
||
| _custom_exts = kwargs.pop('custom_import_extensions', []) or [] | ||
| if isinstance(_custom_exts, (text_type, bytes)): | ||
| _custom_exts = [_custom_exts] |
There was a problem hiding this comment.
hmm, not sure how I feel about a parameter which ends with s that can take a str, let's instead raise a TypeError and require an iterable of some sort
Also let's just require that they are text or ascii compatible:
custom_import_extensions = [
ext.encode('UTF-8')
for ext in kwargs.pop('custom_import_extensions', ())
]
sasstests.py
Outdated
|
|
||
|
|
||
| def test_import_no_css(tmpdir): | ||
| tmpdir.join('other.css').write('body {colour: green}') |
sasstests.py
Outdated
| b'.css', | ||
| [b'.css'], | ||
| ['.foobar', '.css'], | ||
| ['.foobar', '.css', b'anything'], |
There was a problem hiding this comment.
I think this case is unreasonable 🤷♂️
|
Done, except I'm now concerned this is just a sticking plaster over the problem. It currently causes the warning So this change will be broken again in libsass 3.6.0. I guess the long-term solution is for libsass-python to provide a css loader. Perhaps we should change the api now to a |
From sass/libsass#2636 (comment) it seems that the warning will disappear in 3.6.0 so I think this is fine? @xzyfer can you confirm that the this approach will continue to work in 3.6.0? |
|
@samuelcolvin now that the dust has settled, is this good to go? if so I'll merge and make a release |
|
Yes, I think good to go. |
|
This has been released as part of 0.14.4 -- thanks again for the fix! |
|
Great, I'll update my tool grablib which uses libsass extensively now. |
|
just a heads up @samuelcolvin -- looks like upstream has backtracked on the |
|
Great news. Very good news they agreed to revert this. Do we need to do anything here? |
|
Nope! I'll handle it next time we pull in upstream :) note that in the next version there will be a deprecation warning and some time after that the options will be removed but no immediate action (and I'll ping you again when that's happening 🎉) |
|
To summaries the plan moving forward. It's become clear that people rely on being able to resolve Specifically the behaviour we want to deprecate is the using Sass language features in |
|
@xzyfer thanks for the great communication on this 👍 |
3 participants
fix #245, ref sass/libsass@e690924