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

Look at ESLint #1

Closed
Snugug opened this issue Feb 16, 2015 · 16 comments
Closed

Look at ESLint #1

Snugug opened this issue Feb 16, 2015 · 16 comments

Comments

@Snugug
Copy link
Member

Snugug commented Feb 16, 2015

So, I have no idea how to write a linter, but I think this should probably be based off of ESLint, so I'm going to do that.

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

I have some experience in this area. Long story short you need to parse the source file into an AST tree . This produces a data structure which we can easily compare our rules against.

First things first we'll need a parser. There unfortunate aren't many parsers for Sass outside of the language implementations. I've been meaning to write one for a while now.

The one I am aware of is https://www.npmjs.com/package/gonzales-pe. It might also be worth looking to if/how scss-list is generating it's AST tree.

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Yah; I've played a little bit with lexers and parsers (but totally forgot about them without Googling!) Good to see that's the path I should be on!

I've found https://github.com/floby/node-parser, https://github.com/shfx/node-lexer, and https://github.com/aaditmshah/lexer (which looks the most promising to me).

One question think about; do we want this to be a strict port of scss-lint, or do we simply want to be compatible with it? I may lean towards the later as it would allow for things like a plugin system and, instead of just warning about property order, actually updating the source file with the respected ordering (or provide a more robust message that tells you what your property order actually needs to be)

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

I'd opt for the latter. Taking the ESLint approach of plugable rules is the mecca IMHO.

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Okay, good. We should change the name then so there isn't confusion. Maybe sass-lint?

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

I'm not sure how to approach this. I personally wanted to focus on the scss syntax. IMHO the old sass syntaxes adoption is declining and I'm not particularly keen on having two parsers initially.

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Oh, fully agree focus on scss first; I'm thinking Sass (the language), not .sass

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Sass lint?

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

Ah yeah, I meant to add I wanted to avoid potentially confusion, but maybe that's not an issue. Most people probably associate sass with .scss. sass-lint is probably fine, but I also think node-sass-lint is fine :) We can always change if there is confusion.

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

I know I do. I'm going to go with that. If for no other reason than scss-lint already exists in npm as node bindings for the Ruby version. FWIW, Gonzales appears to work with .sass too

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

👍

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

I say we go lower case. Capital case is really awkward to me.

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Yah, it's lower case

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

The repo is currently Sass-lint

@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Doh!

@xzyfer
Copy link
Member

xzyfer commented Feb 17, 2015

🍰 🎉

@Snugug Snugug closed this as completed Feb 17, 2015
@Snugug
Copy link
Member Author

Snugug commented Feb 17, 2015

Closed in favor of #2

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

No branches or pull requests

2 participants