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

Add spec for invisible custom property #1249

Closed
wants to merge 1 commit into from

Conversation

xzyfer
Copy link
Contributor

@xzyfer xzyfer commented Apr 16, 2018

This highlighted a bug in LibSass.

@nex3 do think custom properties be invisible? My feeling is that they should be output regardless.

Note this currently causes an error in Ruby Sass

NoMethodError: undefined method `size' for nil:NilClass
  Use --trace for backtrace.

[skip ruby-sass]
[skip libsass]

xzyfer added a commit to xzyfer/libsass that referenced this pull request Apr 16, 2018
xzyfer added a commit to sass/libsass that referenced this pull request Apr 16, 2018
xzyfer added a commit to sass/libsass that referenced this pull request Apr 16, 2018
@nex3
Copy link
Contributor

nex3 commented Apr 19, 2018

--foo:; should be a syntax error. The spec requires a <declaration-value> production, which requires at least one token of some kind. Ruby Sass's behavior is obviously still incorrect.

There's still a question of what to do for --foo:#{""};. We could either drop the property or error out when we detect that the resolved value is empty; my instinct is to do the latter, since it's almost always going to be an error, and I don't want people to end up writing --foo:#{...} instead of --foo: #{...} on purpose to activate the dropping behavior.

nex3 added a commit that referenced this pull request May 31, 2018
Unlike #1249, this forbids empty custom properties.

Closes #1249
@nex3 nex3 closed this in #1259 Jun 1, 2018
nex3 added a commit that referenced this pull request Jun 1, 2018
Unlike #1249, this forbids empty custom properties.

Closes #1249
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.

None yet

2 participants