Support ignoring rules outside of TTL

[emilgoldsmith]

This would resolve #64

I know I'm bombarding with PRs at the moment @ismay haha, don't worry I'm not in a hurry, just guess I got a bit motivated and excited ;).

This is still a WIP, especially tests, but I thought I would just put this here as a complement to #64 so you could see what my idea was.

A few thoughts I had / decisions I made:

• Are there any indentation stylelint rules I should be careful of not triggering with this? Or I should probably also research stylelint rules concerning comments to make sure I don't introduce any edge cases there
• With this approach we also support single-line comments such as // comment which it wouldn't in normal CSS as we convert them all to /* comment */ comments which I guess I'm also quite fine with if people prefer that that they have that possibility.

Todo:

• Add comment about why space is there in regex so it's clear that we don't input stylelint-disable-line and stylelint-disable-next-line as they should only be used within css blocks
• Add thorough, relevantly structured tests (valid / invalid isn't a suitable structure for this code)
• Add note to README

[mxstbr]

I quite like this approach as we don't have to do much and can let stylelint handle everything! 😜

[emilgoldsmith]

I quite like this approach as we don't have to do much and can let stylelint handle everything! 😜

Haha yes, that's the plan! ^^

[emilgoldsmith]

Hmmm... I'm ready for review, but prettier is acting up, it really doesn't like styled-components haha.

Travis CI was failing because of it, so I let prettier do it's thing, but while it on the one hand onelines a lot of the css that just has one declaration (which I guess works, but isn't that neat) the really bad thing is it adds semicolons in plain wrong places that probably breaks the tests (we'll see in a sec <.<).

I think the easiest thing is to maybe just make prettier ignore the fixtures folder where we keep all our styled-components syntax? As I have any way been adding // prettier-ignore statements before all of my styled-components stuff atm, which is really tedious.

[emilgoldsmith]

Okay I implemented my proposed exclusion of test/fixtures from prettier linting and removed all prettier-ignore statements with a quick regex on the folder, Now hopefully all the tests should pass.

Feel free to tell me to reverse the prettier exclusion if you want.

I'll go get some dinner and come back to hopefully see a passed travis build :).

[emilgoldsmith]

Okay... Going to dinner now! And this time it'll work :P.

I had missed also changing .lintstagedrc

[emilgoldsmith]

Okay, now I'm just really confused. I need some help with this I'm afraid, Travis says lint:prettier fails, but it doesn't on my machine. I even tried doing git reset --hard onto the remote to make sure I had the exact same

[ghost]

Okay, now I'm just really confused. I need some help with this I'm afraid, Travis says lint:prettier fails, but it doesn't on my machine. I even tried doing git reset --hard onto the remote to make sure I had the exact same

@emilgoldsmith I've checked out your branch, and for me it fails where it fails for travis as well, src/parsers/index.js. You can fix it by running:

./node_modules/.bin/prettier --list-different --print-width 100 --tab-width 2 --no-semi --single-quote --write '{src/**/*.{js,jsx},test/*.{js,jsx}}'

From the root of the project. That fixed it for me. Don't know if you want me to commit this to your branch, so I'm listing it here. No idea why it doesn't fail on your machine though. Maybe a different version of prettier?

[emilgoldsmith]

Hey @ismay thanks, yeah tried

rm -Rf node_modules
npm install

and then it suddenly found it. Mysterious indeed. I do believe that I have upgraded my node and npm versions since first installing the repo, so that could possibly be the issue (as in it was installed with different node version?) that kind of stuff sometimes causes issues. But definitely was mysterious!

When I did npm install it updated package-lock.json. I have npm v5.1 I don't know if it does some smart optimizations on install? I pushed it anyway as I assumed npm knew what it was doing, but I can remove that if you guys think so.

[coveralls]

Coverage increased (+0.04%) to 99.408% when pulling ef173d6 on emilgoldsmith:feature/#64/support-ignoring-rules-outside-TTL into 2786011 on styled-components:master.

[emilgoldsmith]

Yay! Finally :P.

Yep, so this is now, finally, 100% ready for review.

[coveralls]

Coverage increased (+0.04%) to 99.408% when pulling cb19500 on emilgoldsmith:feature/#64/support-ignoring-rules-outside-TTL into 2786011 on styled-components:master.

[mxstbr]

This looks awesome, amazing job! 👏

Can't find anything I'd like to change, @ismay?

[ghost]

Nope, me neither, looks really good. Great job @emilgoldsmith !

[mxstbr]

Somebody publish a new version? I'm not setup locally right now 😊

[emilgoldsmith]

Don't we need some kind of permissions for that?

[emilgoldsmith]

Remember to publish the newest version at some point :).

If you want I can also do it if you give me permissions, up to you, I'm also emilgoldsmith on npm anyway.

[ghost]

@emilgoldsmith We have permissions now:

╰─$ npm owner ls
emilgoldsmith <[email protected]>
ismay <[email protected]>
mxstbr <[email protected]>

[emilgoldsmith]

Awesome, well, in that case I guess I'll go ahead and publish what we have now!

[emilgoldsmith]

Done!

And just pushed a v0.2.0 tag as well.

For the future maybe we shouldn't update the package.json beforehand as we did in #60 as then we can't use tools like np which are really neat, but they upgrade the package.json for you and don't allow you to publish the current version of the package.json.

So just did it manually this time :).