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

Added PostCSS Support [fixes #37] #80

Merged
merged 5 commits into from Jul 17, 2017

Conversation

Projects
None yet
8 participants
@lordgiotto
Contributor

lordgiotto commented Jan 28, 2017

Requirements

  • ✓ Filling out the template is required. Any pull request that does not include enough information to be reviewed in a timely manner may be closed at the maintainers' discretion.
  • ✓ All new code requires tests to ensure against regressions

Description of the Change

I added PostCSS scope checks to provider, in order to enable language-postcss to be autocompleted through this provider.

Alternate Designs

The alternative could be use language-css while using PostCSS, but the programming experience would be really awful.

Benefits

Enables language-postcss to be completed trough autocomplete-css provider. Everybody who use postcss can take advantages of both postcss syntax highlight and autocompletition.
Fixes #37.

Possible Drawbacks

PostCSS is not a real syntax, but a collection of plugins: it's impossible to anticipate the exact configuration of the user, so the PostCSS support should be improved gradually to cover most of the possible scenarios.

Applicable Issues

There is room for improvement: a better veriables support, prevents display: var(--b) to be autocompleted in display: var(--block) (now it's a general problem, the same happens with display: $b -> display: $block)

@pixxx

This comment has been minimized.

Show comment
Hide comment
@pixxx

pixxx Feb 25, 2017

This works for me, just saying! And thanks!!!

pixxx commented Feb 25, 2017

This works for me, just saying! And thanks!!!

@Dictory

This comment has been minimized.

Show comment
Hide comment
@Dictory

Dictory Feb 26, 2017

Ok, and how long we need wait for release?

Dictory commented Feb 26, 2017

Ok, and how long we need wait for release?

@pixxx

pixxx approved these changes Mar 6, 2017

Still works for me after updating 0.15.1. No more travis.yml file the rest is the same.

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Mar 6, 2017

Member

This is on my TODO list. Have some other things I'd like to get to first though.

Member

50Wliu commented Mar 6, 2017

This is on my TODO list. Have some other things I'd like to get to first though.

@lordgiotto

This comment has been minimized.

Show comment
Hide comment
@lordgiotto

lordgiotto Apr 27, 2017

Contributor

Please let me know if a rebase (or some modifications) is eventually needed when you will decide to merge :)

Contributor

lordgiotto commented Apr 27, 2017

Please let me know if a rebase (or some modifications) is eventually needed when you will decide to merge :)

Show outdated Hide outdated lib/provider.coffee
Show outdated Hide outdated spec/provider-spec.coffee
Show outdated Hide outdated lib/provider.coffee
Show outdated Hide outdated lib/provider.coffee
@Dictory

This comment has been minimized.

Show comment
Hide comment
@Dictory

Dictory May 31, 2017

I tested it 2 days and did not see any bags. Only specific moment for postscss plugins, for example:
i have postcss-short and can use position: absolute 0 0 0 0, while i print something after position value whatever atom show me about 'absolute'. (sorry for my English, that heavy to explain)

Dictory commented May 31, 2017

I tested it 2 days and did not see any bags. Only specific moment for postscss plugins, for example:
i have postcss-short and can use position: absolute 0 0 0 0, while i print something after position value whatever atom show me about 'absolute'. (sorry for my English, that heavy to explain)

@Dictory

I tested it 2 days and did not see any bags

@lordgiotto

This comment has been minimized.

Show comment
Hide comment
@lordgiotto

lordgiotto Jun 2, 2017

Contributor

@50Wliu I made all requested changes (despite what github says) ;)
Can I do something else, or that's good to you?

Contributor

lordgiotto commented Jun 2, 2017

@50Wliu I made all requested changes (despite what github says) ;)
Can I do something else, or that's good to you?

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Jun 2, 2017

Member

Yep, I'm aware. I will be taking another look at this again shortly.

but I thought that probably it's the scope it should use, so I left it to be future-proof

You could submit a PR to language-postcss changing it 😉. We've been working on trying to standardize most of the CSS scopes lately, which is why there's lots of TODOs in autocomplete-css to remove old scopes.

Also, I just merged a PR, and it looks like there's conflicts again. Sorry about that!

Member

50Wliu commented Jun 2, 2017

Yep, I'm aware. I will be taking another look at this again shortly.

but I thought that probably it's the scope it should use, so I left it to be future-proof

You could submit a PR to language-postcss changing it 😉. We've been working on trying to standardize most of the CSS scopes lately, which is why there's lots of TODOs in autocomplete-css to remove old scopes.

Also, I just merged a PR, and it looks like there's conflicts again. Sorry about that!

@yzalvov

This comment has been minimized.

Show comment
Hide comment
@yzalvov

yzalvov Jun 4, 2017

Hey guys. Sorry for a newby question. But how can I install this PR on my local version of Atom? So I could use and test it. Thank you very much in advance!
@lordgiotto and thank you very much!

yzalvov commented Jun 4, 2017

Hey guys. Sorry for a newby question. But how can I install this PR on my local version of Atom? So I could use and test it. Thank you very much in advance!
@lordgiotto and thank you very much!

@lordgiotto

This comment has been minimized.

Show comment
Hide comment
@lordgiotto

lordgiotto Jun 4, 2017

Contributor

@50Wliu Rebased 😉
@whynotthough You are welcome 😄 Simply clone my repo into ~/dev/packages/autocomplete-css and switch to postcss branch: then start atom in development mode withatom -d

Contributor

lordgiotto commented Jun 4, 2017

@50Wliu Rebased 😉
@whynotthough You are welcome 😄 Simply clone my repo into ~/dev/packages/autocomplete-css and switch to postcss branch: then start atom in development mode withatom -d

@Dictory

This comment has been minimized.

Show comment
Hide comment
@Dictory

Dictory Jul 10, 2017

any updates?

Dictory commented Jul 10, 2017

any updates?

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Jul 10, 2017

Member

No updates. Still on my radar.

Member

50Wliu commented Jul 10, 2017

No updates. Still on my radar.

@lordgiotto

This comment has been minimized.

Show comment
Hide comment
@lordgiotto

lordgiotto Jul 11, 2017

Contributor

@50Wliu Can I do something more? I noticed github erroneously displays that there's still one change to fulfill your review, but that's not true :)

Contributor

lordgiotto commented Jul 11, 2017

@50Wliu Can I do something more? I noticed github erroneously displays that there's still one change to fulfill your review, but that's not true :)

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Jul 12, 2017

Member

Yeah, I hope to re-review this by the end of the week, at which point I'll update the "changes requested" status if necessary. Just got back from vacation and am still catching up on what happened while I was gone.

Member

50Wliu commented Jul 12, 2017

Yeah, I hope to re-review this by the end of the week, at which point I'll update the "changes requested" status if necessary. Just got back from vacation and am still catching up on what happened while I was gone.

@50Wliu

50Wliu approved these changes Jul 17, 2017

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Jul 17, 2017

Member

Looks pretty low-risk to me. Thanks @lordgiotto! However, please note that you'll probably still be responsible for the maintenance of this feature since the Atom team doesn't use or maintain language-postcss.

Member

50Wliu commented Jul 17, 2017

Looks pretty low-risk to me. Thanks @lordgiotto! However, please note that you'll probably still be responsible for the maintenance of this feature since the Atom team doesn't use or maintain language-postcss.

@50Wliu 50Wliu merged commit b34394e into atom:master Jul 17, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Dictory

This comment has been minimized.

Show comment
Hide comment
@Dictory

Dictory Jul 18, 2017

@lordgiotto, @50Wliu thanks for your work! My congratulations.

Dictory commented Jul 18, 2017

@lordgiotto, @50Wliu thanks for your work! My congratulations.

@Zalaski

This comment has been minimized.

Show comment
Hide comment
@Zalaski

Zalaski Aug 13, 2017

Hi all,

I'm still having this issue; when PostCSS is my syntax, autocomplete is not functioning for properties. I understand a fix was implemented here, and I've updated to Atom 1.19.0 on MacOS, so maybe I've missed a step.

Autocomplete works again if I set the syntax to CSS, but then syntax for mixins are no longer highlighted.

I'm relatively new to web development, so please let me know what more information you would need from my end, as I'm not sure what to provide as of right now.

Any help would be greatly appreciated.

Zalaski commented Aug 13, 2017

Hi all,

I'm still having this issue; when PostCSS is my syntax, autocomplete is not functioning for properties. I understand a fix was implemented here, and I've updated to Atom 1.19.0 on MacOS, so maybe I've missed a step.

Autocomplete works again if I set the syntax to CSS, but then syntax for mixins are no longer highlighted.

I'm relatively new to web development, so please let me know what more information you would need from my end, as I'm not sure what to provide as of right now.

Any help would be greatly appreciated.

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Aug 13, 2017

Member

@Zalaski I believe PostCSS support is only on Atom 1.20.0 beta.

Member

50Wliu commented Aug 13, 2017

@Zalaski I believe PostCSS support is only on Atom 1.20.0 beta.

@Zalaski

This comment has been minimized.

Show comment
Hide comment
@Zalaski

Zalaski Aug 13, 2017

@50Wliu Thanks for clarification, could you suggest how I can implement the changes on my end? I'm also new to Git, but I'm assuming Pull request/config changes, or would you advise that I just wait till 1.20.0 enters the Stable channel?

Zalaski commented Aug 13, 2017

@50Wliu Thanks for clarification, could you suggest how I can implement the changes on my end? I'm also new to Git, but I'm assuming Pull request/config changes, or would you advise that I just wait till 1.20.0 enters the Stable channel?

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Aug 13, 2017

Member

I'd advise just waiting.

Member

50Wliu commented Aug 13, 2017

I'd advise just waiting.

@Dictory

This comment has been minimized.

Show comment
Hide comment
@Dictory

Dictory Aug 21, 2017

@Zalaski you can update autocomplete directly.

Dictory commented Aug 21, 2017

@Zalaski you can update autocomplete directly.

@50Wliu

This comment has been minimized.

Show comment
Hide comment
@50Wliu

50Wliu Aug 21, 2017

Member

@Dictory I wouldn't recommend that unless you know exactly what you're doing - that will override the bundled version, which commonly causes hard-to-diagnose bugs.

Member

50Wliu commented Aug 21, 2017

@Dictory I wouldn't recommend that unless you know exactly what you're doing - that will override the bundled version, which commonly causes hard-to-diagnose bugs.

@Dictory

This comment has been minimized.

Show comment
Hide comment
@Dictory

Dictory Aug 22, 2017

@50Wliu yes, you are right. I just wanted say about that opportunity.

Dictory commented Aug 22, 2017

@50Wliu yes, you are right. I just wanted say about that opportunity.

@jessy1092

This comment has been minimized.

Show comment
Hide comment
@jessy1092

jessy1092 Sep 13, 2017

Works perfect on Atom 1.20. Thanks @50Wliu @lordgiotto 💯

jessy1092 commented Sep 13, 2017

Works perfect on Atom 1.20. Thanks @50Wliu @lordgiotto 💯

@rodrigojcmello

This comment has been minimized.

Show comment
Hide comment
@rodrigojcmello

rodrigojcmello Sep 14, 2017

no sugarss support? 😢

rodrigojcmello commented Sep 14, 2017

no sugarss support? 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment