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

stylelint fix findings #122

Closed
bardware opened this issue May 31, 2017 · 20 comments
Closed

stylelint fix findings #122

bardware opened this issue May 31, 2017 · 20 comments
Labels
Enhancement ✨ New feature or request Help wanted 🙋 Help is needed
Milestone

Comments

@bardware
Copy link

bardware commented May 31, 2017

Hi,

is there a possibility to have findings like whitespaces in comments as specified in rule scss/double-slash-comment-whitespace-inside fixed automatically?

@bardware bardware changed the title stylelint --fix argument stylelint fix findings May 31, 2017
@kristerkari
Copy link
Collaborator

I haven't had a lot of time for this project lately, so I'm not sure how stylelint's autofixing is implemented. Would be interesting to know if it is easy to implement fixing for plugins.

Any ideas @jeddy3 @davidtheclark @evilebottnawi ?

@kristerkari kristerkari added the Enhancement ✨ New feature or request label Jun 28, 2017
@jeddy3
Copy link
Collaborator

jeddy3 commented Jun 28, 2017

stylelint-order is the first plugin (pack) I know of to add support for --fix.

Enabling plugins to autofix their violations is one of the driving forces behind adding the feature to stylelint.

@kristerkari kristerkari added the Help wanted 🙋 Help is needed label Jul 23, 2017
@OriR
Copy link
Collaborator

OriR commented Mar 7, 2018

@kristerkari I'm integrating stylelint-scss to an existing project and I'm getting a hundreds of errors on similar rules.

Is there any chance we can promote this?

@kristerkari
Copy link
Collaborator

@OriR That sure is quite frustrating.

I might have some time in the near future to look at implementing --fix for some of the rules, but there are quite many of them and I'm limited on the time that I can put to this library.

Any help to add fixing to rules and any ideas how to get people to help is appreciated. :)

@OriR
Copy link
Collaborator

OriR commented Mar 7, 2018

any ideas how to get people to help is appreciated. :)

I'm a big believer in just asking nicely ;)

So I'm here to help! let me know how you want to divide the work, and if you could point me at the right direction so I'll start poking around the code I'll get right on it 😃

@kristerkari
Copy link
Collaborator

kristerkari commented Mar 7, 2018

I'm very happy that you want to help with this @OriR 🙏So thanks in advance!

First of all, there are some rules that can be too complicated to implement --fix, so we need to find out which ones can support it.

If it helps, we can start gathering a list of rules that can support fixing.

It's very easy to add unit tests for the fix flag, you just need to enable it in tests (fix: true) and then add fixed: next to a test that is rejected:
https://github.com/stylelint/stylelint/blob/master/lib/rules/no-missing-end-of-source-newline/__tests__/index.js#L9-L43

The code to do the fixing can be very simple or somewhat complicated depending on the rule. You can use if (context.fix) { to see if the fixing is enabled. Here's an example:
https://github.com/stylelint/stylelint/blob/master/lib/rules/no-missing-end-of-source-newline/index.js#L25-L29

@kristerkari
Copy link
Collaborator

@OriR
Copy link
Collaborator

OriR commented Mar 8, 2018

I took a quick look at the readme and I think the --fix flag could apply to these rules:

Rules that I'm not sure about are:

  • selector-no-redundant-nesting-selector - although redundant it may change the actual selector output of the rule.
  • double-slash-comment-inline - may cause subsequent errors if we just lift up the comment to the line above, so it may be possible but need to be done carefully.
  • at-import-no-partial-leading-underscore - technically we can "fix" it but it may cause errors if there's already a file with the same name without the leading underscore so I'd rather not take any risks about this.

The rest are "pattern" or "missing" rules that I don't think are even possible to automatically fix.

I think this is a good place to start from, WDYT @kristerkari ?

@kristerkari kristerkari added this to the --fix milestone Mar 8, 2018
@kristerkari
Copy link
Collaborator

kristerkari commented Mar 8, 2018

@OriR That list looks really good! Thanks!

The ones you are not sure about are rules that most likely can not have autofixing and the same goes for "pattern" and "missing" rules.

I created a new milestone called "--fix", so we can start implementing autofix to rules one-by-one. It's probably a good idea to update your list when working on a rule by marking (WIP @username) next to the rule name.

@kristerkari
Copy link
Collaborator

@OriR I started with an easy one:
#221

@OriR
Copy link
Collaborator

OriR commented Mar 9, 2018

@kristerkari Wow!
Wasn't expecting for such a quick turnaround!
Was gonna try and start working on it this weekend but I don't think I'd be able to learn the codebase & PostCSS AST in time because it looks like you're already halfway through 😅

@kristerkari
Copy link
Collaborator

Haha, don't worry, there are still plenty of rules that need autofix. You can have a look at my pull requests for example of how I have implemented autofixing :)

@OriR
Copy link
Collaborator

OriR commented Mar 9, 2018

Yeah sure!
I'll start poking around and see what I come up with, I'll probably open my first PR sometime tomorrow :)

@kristerkari
Copy link
Collaborator

btw. operator-no-* rules might be a bit tricky to autofix, we need to have a closer look and decide if we want to do it or not.

@OriR
Copy link
Collaborator

OriR commented Mar 9, 2018

Yeah, saw that 😞
Also, the function findCommentsInRaws makes it all the more difficult to fix double-slash-comment-whitespace-inside and operator-no-unspaced because it doesn't actually traverses the AST, so we can't just modify a node we find that needs fixing during the same iteration.

@kristerkari
Copy link
Collaborator

We can maybe skip the rules that use findCommentsInRaws for now, and later refactor the code to better support autofixing.

@kristerkari
Copy link
Collaborator

This has been released in 3.0.0. Thanks for helping!

@PurpleTape
Copy link

Hello!
May you tell me if the autofix feature is currently available? The matter is that at me the rule "scss/double-slash-comment-whitespace-inside" is not applied (autofix //comment to // coomment). What to do in such a situation?
Thanks!

@wuyuweixin
Copy link

Not fixed yet

@Sergiobop
Copy link

Not fixed @kristerkari , the problem @PurpleTape says is still happening

Hello! May you tell me if the autofix feature is currently available? The matter is that at me the rule "scss/double-slash-comment-whitespace-inside" is not applied (autofix //comment to // coomment). What to do in such a situation? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement ✨ New feature or request Help wanted 🙋 Help is needed
Development

No branches or pull requests

7 participants