-
Notifications
You must be signed in to change notification settings - Fork 89
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 tests for the indented syntax #2034
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please take a look at the style guide for HRX tests, in particular the sections on irrelevant placeholders and testing one thing per spec. I think a fair number of these are more complex than they need to be in an attempt to test realistic CSS, but here terse is better than realistic.
I recommend adding some error tests as well to cover cases that are invalid, especially ones that may differ in the new system.
@nex3 Do you have a preference between adding tests to a new |
@nex3 I've made changes per the style guide, and merged in with the existing file structure. I also found https://github.com/sass/sass-spec/tree/main/spec/non_conformant/sass and wasn't sure if it would make sense to have a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can generally ignore the non-conformant tests, although if you notice that you're writing tests that cover all the same cases go ahead and delete the redundant non-conformant tests.
|
||
<===> sass/tab/output.css | ||
a { | ||
--b: c; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this behavior currently differs between the syntaxes- in a custom property, trailing whitespace is removed in Indented and condensed in Scss (see line 20).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a design conflict here between "trailing whitespace in custom property values is observable in CSS" and "end-of-line trailing whitespaces is invisible and generally shouldn't matter", complicated further by the fact that custom property trailing whitespace nearly never matters in practice. I think given that we're planning to allow --b: c ;
in the indented syntax eventually anyway, preserving the existing behavior is probably the right way forward.
This adds minimal coverage to the indented syntax, with focus on the areas that may change with upcoming improvements, specifically line breaks and white space parsing.