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 test for @at-root feature #59

Merged
merged 1 commit into from
Oct 6, 2014
Merged

Conversation

lunelson
Copy link
Contributor

@lunelson lunelson commented Oct 6, 2014

A tentative test suite for @at-root, covering the cases I can think of

sass/libsass#353

HamptonMakes added a commit that referenced this pull request Oct 6, 2014
@HamptonMakes HamptonMakes merged commit 55e0dfe into sass:master Oct 6, 2014
@lunelson lunelson deleted the issue-353 branch October 6, 2014 18:19
@KittyGiraudel
Copy link
Contributor

For sass-compatibility, we are running some tests from sass-spec, including this one. Unfortunately, Sass 3.3 chokes on this test because of the keyframe directive, which is completely unrelated to @at-root whatsoever.

Consider this test:

.baz {
  color: red;

  @keyframes kf2 {
    from: 0;
    to: 100;
  }
}

Sass 3.3:

.baz {
  color: red;
}
@keyframes kf2 {
  .baz {
    from: 0;
    to: 100;
  }
}

Sass 3.4:

.baz {
  color: red;
}
@keyframes kf2 {
  from: 0;
  to: 100;
}

Sass 3.3 does not output a nested @keyframes block at root level. It has nothing to do with the @at-root directive, which is perfectly supported by Sass 3.3.

Could we, by any chance, remove this nested @keyframes block from this test so Sass 3.3 can report positive, as it should do?

@xzyfer
Copy link
Contributor

xzyfer commented Dec 14, 2014

I agree for a couple different reasons.

@at-root is a large feature much like maps or @extends. With that in mind I say we create a spec/libsass/at-root folder with a series of granular cases. This will help greatly with debugging and implementing the feature.

I also think @hugogiraudel's sass-compatibility project (or something like it) is important for the community. As I've discussed on the sass-compatibility project feature support is not boolean. It's certainly worth while to know the aspects of a feature and be able to at a glance determine partial implementations.

Did you want to tackle this @hugogiraudel putting together a good series of specs for spec/libsass/at-root?

@KittyGiraudel
Copy link
Contributor

I also think @hugogiraudel's sass-compatibility project (or something like it) is important for the community. As I've discussed on the sass-compatibility project feature support is not boolean. It's certainly worth while to know the aspects of a feature and be able to at a glance determine partial implementations.

@valeriangalliat has done one hell of a job to make it possible for me to pull tests from sass-spec, while still being able to have my own tests when necessary. Along the same lines, sass-compatibility will soon provide a "mixed state", where it's neither fully supported nor completely absent. With this will come a way to know what tests passed and what tests failed.

Did you want to tackle this @hugogiraudel putting together a good series of specs for spec/libsass/at-root?

Count me in.

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