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

Add autofix support for CSS with standard syntax #2886

Merged
merged 4 commits into from Sep 21, 2017

Conversation

4 participants
@gucong3000
Member

gucong3000 commented Sep 13, 2017

Which issue, if any, is this issue related to?

no

Is there anything in the PR that needs further explanation?

Autofix css standard syntax error by postcss-safe-parser.

@evilebottnawi

Looks good, btw i recommend use pify instead fs-extra for promising.

@jeddy3

Interesting!

I think we'll need this behaviour documenting and more tests, especially around non-standard syntax.

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 13, 2017

@gucong3000 Thanks!

As per the guide, only use type: labels for issues. The idea is that you should create an issue to discuss a feature first, before submitting a PR. Those issues are then assigned type: labels and PRs should reference the original issue.

@gucong3000 gucong3000 changed the title from Add autofix support for css syntax error to Add autofix support for CSS with standard syntax Sep 14, 2017

@jeddy3

jeddy3 approved these changes Sep 14, 2017

@gucong3000 Thanks. This is an interesting feature!

Thanks for making the changes. LGTM.

For those interested, you can see what syntax changes are fixed in the tests for postcss-safe-parser.

Does anyone else have any thoughts on this before we merge it in?

@@ -85,15 +86,13 @@
"eslint-config-stylelint": "^7.0.0",
"file-exists-promise": "^1.0.2",
"flow-bin": "^0.54.0",
"fs-promise": "^2.0.3",

This comment has been minimized.

@ntwb

ntwb Sep 14, 2017

Member

I know this package has been deprecated in favour of fs-extra per https://www.npmjs.com/package/fs-promise but the pify package has not been added via npm install pify --save-dev to replace fs-promise here in package.json

This comment has been minimized.

@jeddy3

jeddy3 Sep 14, 2017

Member

I believe pify is already available as a dependency

This comment has been minimized.

@ntwb

ntwb Sep 14, 2017

Member

That it is, my bad, sorry 😏

@@ -0,0 +1 @@
a {

This comment has been minimized.

@ntwb

ntwb Sep 14, 2017

Member

There should be a end-of-file new line here.

@gucong3000 gucong3000 force-pushed the fix_css_syntax_error branch from 90149a5 to c90518b Sep 14, 2017

@ntwb

ntwb approved these changes Sep 14, 2017

@gucong3000 gucong3000 force-pushed the fix_css_syntax_error branch from c90518b to ddc4c31 Sep 21, 2017

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 21, 2017

@gucong3000 Sorry about the delay merging this in. I've only had time to triage issues and not look into PRs. With 3 approvals, this can be merged by any of the team :)

@jeddy3 jeddy3 merged commit 9dc7f30 into master Sep 21, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.001%) to 95.666%
Details

@jeddy3 jeddy3 deleted the fix_css_syntax_error branch Sep 21, 2017

@jeddy3

This comment has been minimized.

Member

jeddy3 commented Sep 21, 2017

  • Added: autofix of syntax errors in standard CSS e.g. unclosed braces and brackets (#2886).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment