Add a JSON formatter #606
Add a JSON formatter #606
Conversation
|
👍 |
|
Publish it standalone on npm instead, then we can use it with the api & grunt (soon gulp as well hopefully) at least |
|
@SimenB we have a working fork here, You can use it until this is merged, You'll have to point npm at git+https though. https://github.com/AtomLinter/csslint |
|
Any chance that this could be merged or at least reviewed? I would absolutely love to get rid of AtomLinter/csslint and switch AtomLinter/linter-csslint back to this official repository now that it seems to be active again, however without this PR I can't do that yet. |
|
Are there any changes that need to be made before this can be merged in @frvge? |
|
@Arcanemagus , I'm currently doing mostly quick triages to try to get the project alive again. At first sight your PR looks good. I'll add triage to get some other collaborator's opinion. |
|
bump (I really want to get rid of the fork....) |
| Assert.areEqual("{\"messages\":[],\"stats\":[]}", actual); | ||
| }, | ||
|
|
||
| "Should have no output when quiet option is specified and no errors": function() { |
There was a problem hiding this comment.
Shouldn't you define errors & warnings in your result object to test if quiet has an effect?
What happens if I set quiet: "false" ?
There was a problem hiding this comment.
I'm not sure I understand what you are asking here.
The way JSON.stringify works means that the result object being passed in there would still be output of {"messages":[],"stats":[]} if quiet is not true.
Formatters have nothing to do with parsing the options, if csslint isn't sending in a properly valued quiet option that's a bug in the rest of csslint, right?
There was a problem hiding this comment.
Wow, I just realized what you mean, and that's a silly bug, fixing.
Arcanemagus
force-pushed
the
json-formatter
branch
from
ecd3644 to
c215bc4
Compare
src/formatters/json.js
Outdated
| */ | ||
| formatResults: function(results, filename, options) { | ||
| "use strict"; | ||
| return options.quiet && results.messages.length < 1 ? "" : JSON.stringify(results); |
There was a problem hiding this comment.
jshint complains about JSON being unavailable, and it looks like rightfully so as one of the supported platforms is WSH, which may or may not have this defined, depending on the version being run.
I'm not sure how to solve this though, any pointers?
|
Added a commit specifying As the WSH interface is currently not supported, whether this is an issue or not I'll leave to you guys. |
|
Also as a note: None of the other formatters actually test if they properly output messages if there are ones to display and |
Add a JSON formatter option, allowing easier use in tools that have native modes of parsing this output.
Arcanemagus
force-pushed
the
json-formatter
branch
from
9873f21 to
0fc42bc
Compare
|
Any further changes? Comments? |
|
@XhmikosR I re-ran the tests to verify an issue with another PR and it looks like travis is having issues. Do you know anything about that? |
|
@frvge: it's a known issue with node.js 5.7.0. 5.7.1 should be out today or so, so it will be fixed. |
|
Btw, working on incorporating proper multiple file support here. |
|
There, should properly handle multiple files now. If there are any changes required please let me know. (Should I be updating the |
Output results even if options.quiet is true. Adds a test to verify that this is working properly.
Results from multiple files should be handled in a single JSON object instead of output as multiple objects.
Arcanemagus
force-pushed
the
json-formatter
branch
from
e3a5856 to
828bbd5
Compare
|
Any further changes that need to be made? |
|
Are there any issues still needing to be resolved here? If this PR and #605 weren't forcing linter-csslint to use a custom (unmaintained!) fork of |
|
Bump... |
|
@XhmikosR ? I think it looks good. |
Add a JSON formatter option, allowing easier use in tools that have native modes of parsing this output.