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

Update property sort order #331

Merged
merged 6 commits into from
Nov 17, 2015

Conversation

DanPurdy
Copy link
Member

Updates the property-sort-order rule to ignore non standard CSS properties to allow custom properties as described in #302

Closes #302

Looking for possible feedback on this to see if we think this check should be enabled or disabled by an option flag or whether a whitelist would be preferable to this check.

DCO 1.1 Signed-off-by: Dan Purdy danjpurdy@gmail.com

@DanPurdy
Copy link
Member Author

I should also add some extra text to the documentation here.

@jgthms
Copy link

jgthms commented Oct 19, 2015

This looks great 👍
Thanks!

@DanPurdy
Copy link
Member Author

Going to also add a flag to this to disable this behaviour if needs be. Some might want their 'custom' properties to be ordered.

@bgriffith
Copy link
Member

Torn on whether default value for ignore-custom-properties should be true or false.

On one hand I feel that custom values could be miss-spelt properties and therefore should not be ignored.

On the other a default action of ignoring anything custom would offer ease of use without either side having to take further actions to get this rule working for them.

Thoughts?

@DanPurdy
Copy link
Member Author

I think it should remain as false. The default sort order is alphabetical and so it would provide a consistent approach straight out of the box where as it takes a little extra configuration and reading of the documentation to understand why you may not want that and to understand why your custom or misspelt property isn't being linted.

@bgriffith
Copy link
Member

Fair point. Let's stick with false.

bgriffith added a commit that referenced this pull request Nov 17, 2015
@bgriffith bgriffith merged commit db59d75 into sasstools:develop Nov 17, 2015
@DanPurdy DanPurdy deleted the update-property-sort-order branch December 23, 2015 22:17
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.

None yet

3 participants