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 and fix test cases for emptyLineBeforeUnspecified #90

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hudochenkov
Copy link
Owner

Related to #87

code: `
a {
height: 20px;
/* other props */
Copy link
Contributor

@njbraun njbraun Jul 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you suggesting this should pass with emptyLineBeforeUnspecified: 'always', set? I was under the impression this should fail. Anything that isn't a 'height' or 'width' css property should classify as "unspecified" and therefore should have an empty line before it w/ the above config.

That being said, if something isn't a standard css property (e.g. a comment, or the start of a new css selector block), that creates kind of a natural separation.

Let me know what you are thinking.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We ignore requirement if there is a comment before. Take a look at these tests for emptyLineBefore:

{
description: '7',
code: `
a {
display: none;
/* comment */
position: absolute;
}
`,
},

{
description: '9',
code: `
a {
/* comment */
display: none;
/* comment */
position: absolute;
}
`,
},

They are both accepted when emptyLineBefore: "always" is set for position.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Word “unspecified” in properties-order rule always mean properties which are not specified in a config.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying, I should have a PR up soon for this then.

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