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

Enable LibSass specs for hex colors with alpha #1273

Merged
merged 1 commit into from
Jul 26, 2018

Conversation

xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Jul 26, 2018

[skip libsass]

xzyfer added a commit to xzyfer/libsass that referenced this pull request Jul 26, 2018
A deprecation for the old behaviour landed in sass#2302. This PR removes
the deprecation warning for the old behaviour, and starts parsing
4 and 8 digit hex values as Colors.

Looks like we had support for 8 digit hex colors for a while but
somehow 4 digit support was missed.

Spec sass/sass-spec#1273
xzyfer added a commit to xzyfer/libsass that referenced this pull request Jul 26, 2018
A deprecation for the old behaviour landed in sass#2302. This PR removes
the deprecation warning for the old behaviour, and starts parsing
4 and 8 digit hex values as Colors.

Looks like we had support for 8 digit hex colors for a while but
somehow 4 digit support was missed.

Spec sass/sass-spec#1273

Fixes sass#2674
xzyfer added a commit to sass/libsass that referenced this pull request Jul 26, 2018
A deprecation for the old behaviour landed in #2302. This PR removes
the deprecation warning for the old behaviour, and starts parsing
4 and 8 digit hex values as Colors.

Looks like we had support for 8 digit hex colors for a while but
somehow 4 digit support was missed.

Spec sass/sass-spec#1273

Fixes #2674
@xzyfer xzyfer merged commit e91f5fa into sass:master Jul 26, 2018
@xzyfer xzyfer deleted the enable-hex-alpha-libsass branch July 26, 2018 13:11
@nex3
Copy link
Contributor

nex3 commented Jul 26, 2018

@xzyfer Can you delete LibSass-specific files when marking a spec as ignored for LibSass?

@xzyfer
Copy link
Contributor Author

xzyfer commented Jul 26, 2018 via email

@nex3
Copy link
Contributor

nex3 commented Jul 26, 2018

Also, it looks like this broke Travis: https://travis-ci.org/sass/sass-spec/builds/408503694

@xzyfer
Copy link
Contributor Author

xzyfer commented Jul 27, 2018 via email

@nex3
Copy link
Contributor

nex3 commented Jul 27, 2018

The default precision shouldn't matter; the engine sets the precision explicitly.

@xzyfer
Copy link
Contributor Author

xzyfer commented Jul 27, 2018

I'm not sure what I'm missing then.

def precision
@metadata.precision || 5
end

The default appears to be 5. I don't it being set otherwise in any relevant options.yml. Yet the output in the specs appears to use precision 10.

@nex3
Copy link
Contributor

nex3 commented Jul 27, 2018

Since no other implementation is generating the wrong precision here, it seems like it might be a LibSass bug. Can you roll back this PR while you investigate to get the build green?

@nex3
Copy link
Contributor

nex3 commented Jul 31, 2018

Oh I see what's happening here--I generated the expected test cases using Dart Sass, which always uses precision 10.

xzyfer added a commit to sass/libsass that referenced this pull request Nov 11, 2018
A deprecation for the old behaviour landed in #2302. This PR removes
the deprecation warning for the old behaviour, and starts parsing
4 and 8 digit hex values as Colors.

Looks like we had support for 8 digit hex colors for a while but
somehow 4 digit support was missed.

Spec sass/sass-spec#1273

Fixes #2674
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