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

Stringify paths to unique paths correctly #83

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

hexgnu
Copy link

@hexgnu hexgnu commented Nov 28, 2016

The problem here is that gems like compass-sass and bootstrap-sass will
build their own Importer classes that aren't being properly uniqued.

This will resolve #79

@@ -94,7 +94,9 @@ def imports(path, parent_path)
return glob_imports(base, m[2], parent_path)
end

search_paths = ([parent_dir] + load_paths).uniq
# Compass and other gems us their own special loaders

Choose a reason for hiding this comment

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

us their own => use their own

@hexgnu
Copy link
Author

hexgnu commented Jan 23, 2017

Updated thanks @PikachuEXE

@PikachuEXE
Copy link

But the build failed :S

@hexgnu
Copy link
Author

hexgnu commented Jan 23, 2017

well 💩 I'll look at it

@hexgnu
Copy link
Author

hexgnu commented Jan 23, 2017

It appears that sassc/sass has changed how line_comments show up relative to this test.

It was reporting:
test/dummy/app/assets/stylesheets/css_scss_handler.css.scss
but the spec was asking for
/.+\/sassc-rails\/test\/dummy\/app\/assets\/stylesheets\/css_scss_handler.css.scss/

I did fix it as is but I believe it would actually be better to change the test to check on the first path which seems to make more sense to me.

Though that's up to you @PikachuEXE

@PikachuEXE
Copy link

I know nothing about the test
I am just reminding you the buidl failure :P

The problem here is that gems like compass-sass and bootstrap-sass will
build their own Importer classes that aren't being properly uniqued.

Also this resolves the issue outlined in
sass#79
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.

no implicit conversion of Sass::Importers::Filesystem into String
4 participants