csslint icon indicating copy to clipboard operation
csslint copied to clipboard

Add JSON formatter

Open sergeychernyshev opened this issue 9 years ago • 11 comments

JSON formatter for CSSLint results

sergeychernyshev avatar Feb 25 '16 04:02 sergeychernyshev

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.

sergeychernyshev avatar Feb 25 '16 05:02 sergeychernyshev

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

Arcanemagus avatar Feb 25 '16 18:02 Arcanemagus

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

Arcanemagus avatar Feb 25 '16 18:02 Arcanemagus

@Arcanemagus I can try merging the two implementations.

sergeychernyshev avatar Feb 25 '16 21:02 sergeychernyshev

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.

sergeychernyshev avatar Feb 25 '16 21:02 sergeychernyshev

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

Arcanemagus avatar Mar 01 '16 07:03 Arcanemagus

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

frvge avatar Mar 06 '16 13:03 frvge

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 avatar Mar 07 '16 14:03 sergeychernyshev

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

sergeychernyshev avatar Mar 07 '16 14:03 sergeychernyshev

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

frvge avatar Apr 04 '16 13:04 frvge

@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?

sergeychernyshev avatar Apr 04 '16 13:04 sergeychernyshev