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

Pure Sass support #473

Merged
merged 8 commits into from
Dec 3, 2013
Merged

Pure Sass support #473

merged 8 commits into from
Dec 3, 2013

Conversation

glebm
Copy link
Member

@glebm glebm commented Nov 18, 2013

@thomas-mcdonald What do you think?
This uses custom functions, which is flexible but not ideal for people trying to use this not in the ruby world.

Also exposes $bootstrap-sass-asset-helper (default: true), to allow using outside ruby.

@glebm
Copy link
Member Author

glebm commented Nov 21, 2013

Added $bootstrap-sass-asset-helper, to allow using outside ruby.

@ksheedlo
Copy link

+1, let's get this in

@glebm glebm mentioned this pull request Nov 27, 2013
@glebm glebm merged commit cc2ec7e into twbs:master Dec 3, 2013
@thomas-mcdonald
Copy link
Member

❤️ ❤️ ❤️

Turned out to be quite the elegant solution.

@@ -117,6 +123,10 @@ def convert_to_scss(file)
file
end

def replace_asset_url(rule, type)
replace_all rule, /url\((.*?)\)/, "url(if($bootstrap-sass-asset-helper, twbs-#{type}-path(\\1), \\1))"
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry to kick an old PR, but this if syntax is invalid Sass and blows up node-sass/libsass compilation

Copy link
Member Author

Choose a reason for hiding this comment

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

Awesome that you tried this on node-sass! How does it fail exactly? Would you add a test for compiling with it?

The syntax is valid and is specified here: http://sass-lang.com/documentation/Sass/Script/Functions.html#if-instance_method
It also appears to be supported by libsass: https://github.com/hcatlin/libsass/blob/master/functions.cpp#L959-L961

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, I didn't realize there was an IIF like function in Sass. The actual error is bootstrap/mixins:336: error: non-terminal statement or declaration must end with ';'

Copy link
Contributor

Choose a reason for hiding this comment

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

It does look like the if instance is available in libsass https://github.com/hcatlin/libsass/blob/master/functions.cpp#L959

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to #494

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.

4 participants