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

Fix the index function use to work with new Sass versions. #2

Merged
merged 1 commit into from
Oct 7, 2014

Conversation

jvah
Copy link
Contributor

@jvah jvah commented Oct 7, 2014

On failure index returns now null instead of false, but viewports always
compares to false. This is easy to fix by taking a logical not of the resulting
value. For more information see sass/sass#1127

On failure index returns now null instead of false, but viewports always
compares to false. This is easy to fix by taking a logical not of the resulting
value. For more information see sass/sass#1127
jareware added a commit that referenced this pull request Oct 7, 2014
Fix the index function use to work with new Sass versions.
@jareware jareware merged commit ff172d6 into jareware:master Oct 7, 2014
@jareware
Copy link
Owner

jareware commented Oct 7, 2014

Thanks a bunch! This does break on libsass though but I'd imagine there's 10x more gem users out there. Let's fix that before the next release...

@jvah
Copy link
Contributor Author

jvah commented Oct 7, 2014

Damn, should also test with libsass, and it also sounds like libsass does something weird with booleans. It sounds quite logical that both false and null would result in true after boolean "not" operation, maybe should patch it as well...

@jareware
Copy link
Owner

jareware commented Oct 7, 2014

Yeah libsass is a bit of a moving target still. :/

$ npm test will by default run the suite against both compilers, if available.

@jareware
Copy link
Owner

jareware commented Oct 7, 2014

ping @joelmertanen

@jvah
Copy link
Contributor Author

jvah commented Oct 7, 2014

There is a workaround for libsass presented in sass/libsass#349, it appears that the libsass not is known to be broken. We can use this workaround to make viewports work on all versions at the same time. I'll create a new pull request for this.

jvah added a commit to jvah/viewports that referenced this pull request Oct 7, 2014
The not expression in libsass is somewhat broken, but there is a workaround
presented in sass/libsass#349 which fixes this issue for
us. It makes me feel a bit dirty, but so does a ruby dependency.
jareware added a commit that referenced this pull request Oct 7, 2014
Make change in pull request #2 to also work on libsass.
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