support custom import extension #246
Conversation
sass.py
Outdated
| @@ -239,7 +239,7 @@ def compile_dirname( | |||
| input_filename = input_filename.encode(fs_encoding) | |||
| 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
| @@ -584,6 +596,21 @@ def _get_file_arg(key): | |||
| 'not {1!r}'.format(SassFunction, custom_functions) | |||
| ) | |||
|
|
|||
| _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 👍 |
fix #245, ref sass/libsass@e690924