csscomb / vim-csscomb

CSScomb plugin for Vim. Tool for sorting CSS properties in specific order.
csscomb.com
134 stars 18 forks source link

Updated plugin to use the nodejs csscomb implementation #9

Closed faceleg closed 9 years ago

faceleg commented 9 years ago

Please review and let me know if any changes are required before merging

istro commented 9 years ago

:+1: Thank you! For the time being I just cloned your repo, but this should definitely get addressed soon :-)

istro commented 9 years ago

Not sure how it used to work with old implementation - but on errors would it notify me?

An error (which I also get when csscomb is run in command line) is this:

Please check the validity of the block starting from line #25

23 |     margin-right: $margin-default;
25*|     // We need the extra specification to override bootstrap
26 |     > a {
27 |       padding: $margin-small $margin-tab;

Gonzales PE version: 3.0.0-28
Syntax: scss
CSScomb Core version: 3.0.0

Comment is actually fine I think, but in any case - CLI gives an error, shouldn't plugin also do something other than fail silently?

faceleg commented 9 years ago

You're right - I only tried it on vanilla CSS that passed linting (I use syntastic to lint css, I guess that runs before this plugin).

I'll look at adding error output when I get a chance

istro commented 9 years ago

Yeah, it's fair to assume people would set up lint rules to match their csscomb rules, so I guess it's not that big of a deal. Probably not super high priority to add error notification here...

I actually just set up scss-lint (also uses syntastic) to lint my sass, it's AWESOME, hehe. Today's the cleanup day! now that all the styles are neat and combed lint will prevent me from messing it up :+1:

faceleg commented 9 years ago

One would hope that if someone is using csscomb they also know about linting, yes! But I still think it is a good idea to have the plugin at least output something when it barfs instead of doing it in silence.

tonyganch commented 9 years ago

Ahoj! Sorry for delay, I'll take a look at this pr tomorrow.

faceleg commented 9 years ago

I've addressed the issues you noted @tonyganch!

faceleg commented 9 years ago

@istro I've updated this PR to include error output in case of csscomb failure.

Note that it will output only the following portion of the csscomb output:

Please check the validity of the block starting from line #25

istro commented 9 years ago

:+1:

faceleg commented 9 years ago

Note this PR should fix: #6, #7, #8

tonyganch commented 9 years ago

Thank you so much for this pr!

faceleg commented 9 years ago

No problem, thanks for merging! :dancers: