Skip to content

Add support for passing previous file to importer callback #287

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

Merged
merged 1 commit into from
Feb 14, 2019

Conversation

fdintino
Copy link
Contributor

This change adds feature-parity with node-sass importers, where callbacks are passed both the value of the import directive and the path from which the directive was made. This is particularly useful in cases where the importer needs to resolve relative paths.

I wasn't sure about the best way to go about maintaining backwards-compatibility. Here I added a conditional getargspec call in the importer callback wrapper, but I'm open to any other suggestions.

Copy link
Member

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉 thanks for the PR!

@asottile asottile merged commit 690beb9 into sass:master Feb 14, 2019
@fdintino
Copy link
Contributor Author

@asottile I've come to regret the getargspec here a bit. Specifically, it doesn't work with either __call__ or with an instance of functools.partial. Would you be open to a PR that had something like:

def _importer_callback_wrapper(func):
    num_args = None

    @functools.wraps(func)
    def inner(path, prev):
        path, prev = path.decode('UTF-8'), prev.decode('UTF-8')
        if num_args is None:
            num_args = 2
            try:
                ret = func(path, prev)
            except TypeError:
                num_args = 1
                ret = func(path)
        elif num_args == 2:
            ret = func(path, prev)
        else:
            ret = func(path)
        return _normalize_importer_return_value(ret)
    return inner

@asottile
Copy link
Member

asottile commented Feb 24, 2019

Ah yeah that limitation frequently bites

Maybe just (typed on phone sorry):

try:
    ret = func(path, prev)
except TypeError as e:
    if not ... str(e):  # test to make sure it is the TypeError we expect
        raise
    ret = func(path) 
return ... 

@fdintino
Copy link
Contributor Author

I'm fine with that. The only tricky thing is that the message looks different in python 2 and 3. In python 2 it's

TypeError('foo() takes exactly X argument[s] (2 given)')

whereas in python 3 it's:

TypeError('foo() takes X positional argument[s] but 2 were given')

So I guess we could do something like:

try:
    ret = func(path, prev)
except TypeError as e:
    msg = str(e)
    if not msg.endswith('(2 given)') and not msg.endswith('but 2 were given'):
        raise
    ret = func(path)

@asottile
Copy link
Member

Oof, probably fine to just assume all TypeErrors are arg count errors then and drop the check

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 this pull request may close these issues.

2 participants