8000 Fix error with pseudo-element rule and move lists to data assets by bgriffith · Pull Request #671 · sasstools/sass-lint · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix error with pseudo-element rule and move lists to data assets #671

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

Merged
merged 1 commit into from
May 3, 2016

Conversation

bgriffith
Copy link
Member

Currently there is a bug where the rule tries to use charAt without first checking that it is a string.

Closes #670 and moves lists of pseudo classes and elements to data assets.

DCO 1.1 Signed-off-by: Ben Griffith gt11687@gmail.com

@coveralls
Copy link
coveralls commented May 3, 2016

Coverage Status

Coverage decreased (-0.002%) to 97.044% when pulling 0952393 on bgriffith:feature/pseudo-data into 70f52c7 on sasstools:develop.

path = require('path');

var pseudoElements = yaml.safeLoad(
fs.readFileSync(path.join(__dirname, '../../data', 'pseudoElements.yml'), 'utf8')
Copy link
Member
@DanPurdy DanPurdy May 3, 2016

Choose a reason for hiding this comment

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

Do we really need these in separate files right now? They're not being used anywhere else and with such small datasets it seems unnecessary to force two expensive synchronous file read operations on users.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think they fit the bill. They're both data lists that are likely to change over time and it makes sense to have them in the data/ directory instead of in the rule.

Having just run a quick console.time on it, they only take around ~0.1ms to 0.2ms each so I don't personally feel it's too expensive.

@DanPurdy DanPurdy merged commit e5010bd into sasstools:develop May 3, 2016
@DanPurdy DanPurdy added this to the 1.8.0 milestone Jun 17, 2016
@DanPurdy DanPurdy deleted the feature/pseudo-data branch June 17, 2016 12:53
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
0