WIP: Introduce formatter objects and add a JSON format. #116
Conversation
| def report | ||
| io = | ||
| if @options[:out] | ||
| File.open(@options[:out], 'w') |
There was a problem hiding this comment.
There is no code to handle closing of the file because files are closed when the script ends and I had no idea how to use the block notation without repeating the report statement for both cases. Maybe someone has an idea for that?
| @results.each do |relative_filename, runner| | ||
| runner.offenses.each do |offense| | ||
| io.puts <<~EOF | ||
| #{offense.message}#{Rainbow(' (not autocorrected)').red if options[:autocorrect]} |
There was a problem hiding this comment.
I think, this should be unless options[:autocorrect] from how I understand the original code but I let it as is for now. Can on of the maintainers or the original author shed light on this?
| if stats.corrected > 0 | ||
| corrected_found_diff = stats.found - stats.corrected | ||
| if corrected_found_diff > 0 | ||
| io.puts Rainbow( |
There was a problem hiding this comment.
This previously was a call to warn. Is there a reason why this was written to STDERR? The same holds for the elsif stats.found > 0 case below. I did this because I wasn't sure how to handle this for file outpt.
There was a problem hiding this comment.
I think reporting action results to stderr could cause troubles if you invoke the linter from another procces, exit code 1 should be used to report unexpected errors only.
| linter, message = linter_and_message(offense) | ||
| { | ||
| message: message, | ||
| linter: linter, |
There was a problem hiding this comment.
For now, if a rubocop cop found an offense, this is set to the particular cop name and not to 'rubocop' which is the actual erb-lint-linter. I first thought this is better but now think, maybe it is Ok/better to have 'rubocop' here and let the cop remain in the message. Thoughts on this?
There was a problem hiding this comment.
I am agree with you, it would be nice to identify from where the cop comes and let the cop in the message.
| end | ||
| end | ||
|
|
||
| def offense_corrected?(offense) |
There was a problem hiding this comment.
I'm totally not sure about this. The only thing I found in the code so far is the rubocop-linter setting this key on correction. However, I also observed that corrected offenses seem not to get reported - or I did something wrong for that and need to look that up somewhere else.
|
I am working on an atom linter for erb-lint, the actual text output is very difficult to parse, this PR would be great, congrats. |
| @result = { | ||
| metadata: metadata_hash, | ||
| files: [], | ||
| summary: {offense_count: 0, inspected_file_count: 0} |
There was a problem hiding this comment.
It should be interesting to have an offending files counter.
| summary: {offense_count: 0, inspected_file_count: 0} | |
| summary: { offense_count: 0, inspected_file_count: 0, offending_file_count: 0 } |
| def file_completed(relative_filename, runner) | ||
| @result[:summary][:offense_count] += runner.offenses.count | ||
| @result[:summary][:inspected_file_count] += 1 | ||
| @result[:files] = hash_for_file(relative_filename, runner) |
There was a problem hiding this comment.
We are reporting the results only for the last processed file, and I think it could be better add the files to the final report only if offenses are present.
| @result[:files] = hash_for_file(relative_filename, runner) | |
| if runner.offenses.count.positive? | |
| @result[:summary][:offending_file_count] += 1 | |
| @result[:files] << hash_for_file(relative_filename, runner) | |
| end |
There was a problem hiding this comment.
Oh, careless mistake ...
And I agree, we should only report files with issues. Iirc, the check is currently not necessary because files without offenses do not reach this code anyways. But to record the number of inspected files this would change.
|
Could you please take a look at this commit in my fork? It is based on your work, the idea is:
What do you think? thoughts? |
|
@vzamanillo I've had a look and wrote some comments. |
I took a shot at introducing formatters as suggested in #84 because I'd like this cool gem to become available in syntastic. The one-line-format for #84 could easily be added as another formatter.
Before I go on and add tests I would like to know what you think of this and whether there are suggestions to add/change things.
The JSON format is gratefully adopted from rubocop (without the
:severitykey and:linterinstead of:cop_namein the offense hashes).