Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --format option #159

Merged
merged 24 commits into from
Nov 9, 2020
Merged

Add --format option #159

merged 24 commits into from
Nov 9, 2020

Conversation

andersonkrs
Copy link

@andersonkrs andersonkrs commented Mar 18, 2020

Hey guys!
At my work, we adopted this gem and we are trying to integrate it with this guy: https://github.com/reviewdog/reviewdog in our CI. But, it has been really hard to do, because this tool uses vim's errorformat and it's not that easy to parse the actual output.

I've seen that there are two proposals to tackle this problem: #84 #116, and it hasn't been any movement, so I decided to do this starting point.
This implementation that I've made is strongly inspired by these PRs.

I've added a --format option, with that, users can pass the desired output format. To add a new format is straightforward, you just need to create a new inherited class from Reporter and implement the method show and it will be available on cli automatically.

So, I've refactored and extracted all the code responsible for reporting results in 2 new classes:
1 - MultilineReporter - The default report format
2 - CompactReporter - Oneliner report, outputs data using format: file:line:column: message. This guy will be really useful for many kinds of integrations like CI, editors, etc. 😃

This opens the way to include new output formats more easily, like JSON, XML, etc.

@fidalgo
Copy link

fidalgo commented May 18, 2020

I'm looking into this to configure ale and CI to have a consistent style across all projects.

@rafaelfranca
Copy link
Member

Looks good. Can you document this new option in the README?

@andersonkrs
Copy link
Author

@rafaelfranca Done.

I did one more little thing, I've extracted the responsibility of show preview messages like Linting x files ... into reporter object, because other formats like JSON, XML, etc, can't have that on output.

@ni3t
Copy link

ni3t commented Jul 17, 2020

Hello,

Is there any progress on merging this change? We are migrating our CI to github actions, and the ability to write a custom reporter would make annotating errors in the changed files view a lot easier. No rush if this is still a WIP.

@rafaelfranca rafaelfranca merged commit 6179ee2 into Shopify:master Nov 9, 2020
@andersonkrs andersonkrs deleted the add-custom-formatters branch November 9, 2020 20:34
@exterm exterm temporarily deployed to production January 5, 2021 08:42 Inactive
@brendo
Copy link

brendo commented Jan 8, 2021

With the newly released 0.0.36, this PR seems to introduce an error:

$ bundle exec erblint --lint-all
NoMethodError: undefined method `delegate' for ERBLint::Reporter:Class
Did you mean?  DelegateClass
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/reporter.rb:36:in `<class:Reporter>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/reporter.rb:5:in `<module:ERBLint>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/reporter.rb:4:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint.rb:16:in `require'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint.rb:16:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/cli.rb:3:in `require'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/cli.rb:3:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/exe/erblint:6:in `require'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/exe/erblint:6:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/bin/erblint:23:in `load'
  /tmp/bundle/ruby/2.6.0/bin/erblint:23:in `<top (required)>'

@andersonkrs
Copy link
Author

With the newly released 0.0.36, this PR seems to introduce an error:

$ bundle exec erblint --lint-all
NoMethodError: undefined method `delegate' for ERBLint::Reporter:Class
Did you mean?  DelegateClass
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/reporter.rb:36:in `<class:Reporter>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/reporter.rb:5:in `<module:ERBLint>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/reporter.rb:4:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint.rb:16:in `require'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint.rb:16:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/cli.rb:3:in `require'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/lib/erb_lint/cli.rb:3:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/exe/erblint:6:in `require'
  /tmp/bundle/ruby/2.6.0/gems/erb_lint-0.0.36/exe/erblint:6:in `<top (required)>'
  /tmp/bundle/ruby/2.6.0/bin/erblint:23:in `load'
  /tmp/bundle/ruby/2.6.0/bin/erblint:23:in `<top (required)>'

Oh boy, I'll check this out soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants