search

CSSLint / csslint

Automated linting of Cascading Stylesheets
http://csslint.net
Other
4.77k stars 483 forks source link

Add JSON formatter #642

Open sergeychernyshev opened 8 years ago

sergeychernyshev commented 8 years ago

JSON formatter for CSSLint results

sergeychernyshev commented 8 years ago

Not sure if this implementation is correct, considering that formatResults() returns nothing and all work is done in endFormat().

This works, but feels bad and some users of CSSLint call formatResults directly.

Not sure how to do this better considering that output from multiple files is simply concatenated and wrapped with startFormat() and endFormat() - this doesn't work for JSON, obviously as it requires commas between results.

Arcanemagus commented 8 years ago

Duplicate of #606, but I'd be happy if either of our PR's get accepted...

Arcanemagus commented 8 years ago

This doesn't take into account the quiet option, but then mine doesn't take into account more than one file :stuck_out_tongue:.

sergeychernyshev commented 8 years ago

@Arcanemagus I can try merging the two implementations.

sergeychernyshev commented 8 years ago

Anyone has any idea why travis-ci build is failing? from the logs it doesn't seem like there are any issues with the code, just some permissions issues with the build script.

Arcanemagus commented 8 years ago

Updated #606 with proper support for multiple files, mind looking over it and making sure I'm not missing anything?

frvge commented 8 years ago

Re-ran the tests now that it's Node 5.7.1

sergeychernyshev commented 8 years ago

I looked at #606 and it turns out that both implementations are good on their own accord, but merging them together requires us to resolve issue #645 that I created.

I'll try to take a stab at it soon - will need to introduce new way of handling multi-result formatters.

sergeychernyshev commented 8 years ago

@frvge thank you, I was wondering if it's my code breaking something I don't understand.

frvge commented 8 years ago

@sergeychernyshev , we've merged #606. Is your version still needed?

sergeychernyshev commented 8 years ago

@frvge yes, it is still needed for multi-file support, but it'll have to be updated to merge with support that #606 provided. Should not be pulled as is.

Can you check issue #645 and comment on the issue?