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

Error when declarations are printed without ruleset #2061

Closed
wants to merge 1 commit into from

Conversation

mgreter
Copy link
Contributor

@mgreter mgreter commented Apr 30, 2016

Fixes #1732 - Another one of the category "should error" ❗

Properties are only allowed within rules, directives, mixin includes, or other properties.

Error: Properties are only allowed within rules,
directives, mixin includes, or other properties.
@mgreter mgreter self-assigned this Apr 30, 2016
@mgreter mgreter added this to the 3.3.7 milestone Apr 30, 2016
@xzyfer
Copy link
Contributor

xzyfer commented Apr 30, 2016

Let's not do this. This check does not belong in cssize. The cssize class is a 1-1 port from Ruby, no need to change that.

Ruby solves this a CheckNesting visitor. We an equivalent since there is an entire class of nesting errors we cannot catch properly atm.

@mgreter
Copy link
Contributor Author

mgreter commented Apr 30, 2016

IMO this can be moved over once such a CheckNesting visitor exists. I don't see a reason why this should not be correct, or can you come up with a non failing test?

@xzyfer
Copy link
Contributor

xzyfer commented Apr 30, 2016

This bug doesn't prevent Sass code being ported to LibSass so IMHO it's a high priority. IMO a fix for this bug is not worth the tech debt.

If you really want to fix this it's trivial to add a CheckNesting visitor before cssize runs.

@mgreter mgreter removed this from the 3.3.7 milestone Apr 30, 2016
@mgreter mgreter removed their assignment Apr 30, 2016
@coveralls
Copy link

coveralls commented Apr 30, 2016

Coverage Status

Coverage increased (+0.009%) to 79.087% when pulling ed0e36b on mgreter:bugfix/issue_1732 into c95478c on sass:master.

xzyfer added a commit to xzyfer/libsass that referenced this pull request May 1, 2016
This is introduces a vastly reduced `CheckNesting` visitor from Ruby
Sass as discussed in sass#2061. Doing this properly will require some
small changes to the parser, and probably eval, which currently try
to do some nesting checking of their own.

This is just first step to properly introducing the proper nesting
checking.

Closes sass#2061
Fixes sass#1732
xzyfer added a commit to xzyfer/libsass that referenced this pull request May 1, 2016
This is introduces a vastly reduced `CheckNesting` visitor from Ruby
Sass as discussed in sass#2061. Doing this properly will require some
small changes to the parser, and probably eval, which currently try
to do some nesting checking of their own.

This is just first step to properly introducing the proper nesting
checking.

Closes sass#2061
Fixes sass#1732
xzyfer added a commit to xzyfer/libsass that referenced this pull request May 1, 2016
This is introduces a vastly reduced `CheckNesting` visitor from Ruby
Sass as discussed in sass#2061. Doing this properly will require some
small changes to the parser, and probably eval, which currently try
to do some nesting checking of their own.

This is just first step to properly introducing the proper nesting
checking.

Closes sass#2061
Fixes sass#1732
@xzyfer xzyfer changed the title Error when declarations are printed without ruleset [WIP] Error when declarations are printed without ruleset May 1, 2016
@xzyfer xzyfer changed the title [WIP] Error when declarations are printed without ruleset Error when declarations are printed without ruleset May 1, 2016
@mgreter
Copy link
Contributor Author

mgreter commented May 1, 2016

Superseeded by #2062

@mgreter mgreter closed this May 1, 2016
xzyfer added a commit to xzyfer/libsass that referenced this pull request May 2, 2016
This is introduces a vastly reduced `CheckNesting` visitor from Ruby
Sass as discussed in sass#2061. Doing this properly will require some
small changes to the parser, and probably eval, which currently try
to do some nesting checking of their own.

This is just first step to properly introducing the proper nesting
checking.

Closes sass#2061
Fixes sass#1732
xzyfer added a commit to xzyfer/libsass that referenced this pull request May 2, 2016
This is introduces a vastly reduced `CheckNesting` visitor from Ruby
Sass as discussed in sass#2061. Doing this properly will require some
small changes to the parser, and probably eval, which currently try
to do some nesting checking of their own.

This is just first step to properly introducing the proper nesting
checking.

Closes sass#2061
Fixes sass#1732
xzyfer added a commit to xzyfer/libsass that referenced this pull request May 2, 2016
This is introduces a vastly reduced `CheckNesting` visitor from Ruby
Sass as discussed in sass#2061. Doing this properly will require some
small changes to the parser, and probably eval, which currently try
to do some nesting checking of their own.

This is just first step to properly introducing the proper nesting
checking.

Closes sass#2061
Fixes sass#1732
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants