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

Dynamic formatter configuration broken (v0.36) #2662

Closed
agross opened this issue Jan 18, 2016 · 8 comments
Closed

Dynamic formatter configuration broken (v0.36) #2662

agross opened this issue Jan 18, 2016 · 8 comments

Comments

@agross
Copy link

agross commented Jan 18, 2016

I define my formatters dynamically in my Rakefile to get different output when the check runs on a developer machine vs. a build machine.

This used to work in v0.35.1:

RuboCop::RakeTask.new(:rubocop) do |t|
  t.formatters = %w(files)
  t.formatters << %w(html --out build/rubocop/rubocop.html)
end

And with v0.36 is fails with an error:

$ bundle exec rake rubocop
Running RuboCop...
no implicit conversion of Array into String
C:/Ruby/lib/ruby/2.2.0/optparse.rb:378:in `match'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:378:in `parse_arg'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:531:in `parse'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:1395:in `block in parse_in_order'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:1383:in `catch'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:1383:in `parse_in_order'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:1377:in `order!'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:1469:in `permute!'
C:/Ruby/lib/ruby/2.2.0/optparse.rb:1491:in `parse!'
C:/Ruby/lib/ruby/gems/2.2.0/gems/rubocop-0.36.0/lib/rubocop/options.rb:18:in `parse'
C:/Ruby/lib/ruby/gems/2.2.0/gems/rubocop-0.36.0/lib/rubocop/cli.rb:24:in `run'
...

I added some debug output before options.rb:18 (puts args.inspect):

["--format", "files", "--format", ["html", "--out", "build/rubocop/rubocop.html"]]
@alexdowad
Copy link
Contributor

t.formatters is an array of strings, not an array of arrays of strings.

You can use t.formatters.concat(%w(html --out build/rubocop/rubocop.html)).

Maintainers, please close.

@agross
Copy link
Author

agross commented Jan 18, 2016

Maintainers, please do not close.

@alexdowad The problem with concat is that rubocop prepends --format in front of every array element.

RuboCop::RakeTask.new(:rubocop) do |t|
  puts RuboCop::Version.version
  t.formatters = %w(files)
  t.formatters.concat(%w(html --out build/rubocop/rubocop.html))
end

rubocop v0.36:

$ bundle exec rake rubocop
0.36.0
Running RuboCop...
["--format", "files", "--format", "html", "--format", "--out", "--format", "build/rubocop/rubocop.html"]
No formatter for "--out"
C:/Ruby/lib/ruby/gems/2.2.0/gems/rubocop-0.36.0/lib/rubocop/formatter/formatter_set.rb:84:in `builtin_formatter_class'
C:/Ruby/lib/ruby/gems/2.2.0/gems/rubocop-0.36.0/lib/rubocop/formatter/formatter_set.rb:56:in `add_formatter'
...

rubocop v0.35.1:

$ bundle exec rake rubocop
0.35.1
Running RuboCop...
["--format", "files", "--format", "html", "--format", "--out", "--format", "build/rubocop/rubocop.html"]
No formatter for "--out"
C:/Ruby/lib/ruby/gems/2.2.0/gems/rubocop-0.35.1/lib/rubocop/formatter/formatter_set.rb:93:in `builtin_formatter_class'
...

@alexdowad
Copy link
Contributor

@alexdowad The problem with concat is that rubocop prepends --format in front of every array element.

OK. Thank you for pointing this out. In that case, you can use t.options instead.

t.formatters is just a shortcut for adding strings with --format prepended to options. It is intended to receive an array of strings, each of which is a formatter name.

@alexdowad
Copy link
Contributor

@agross Is your problem resolved?

@agross
Copy link
Author

agross commented Feb 5, 2016

@alexdowad I'm waiting for feedback of the maintainers. I'd like to learn how they think about my approach that was valid for 0.35.1, but broke in 0.36.

@jonas054
Copy link
Collaborator

jonas054 commented Feb 6, 2016

Here's what I think. The --format and --out options are closely tied together, so it's better if we can support @agross's way of defining the options the way it worked in v0.35.1. This should also be documented in the README.

We need to revert the change of this line.

@alexdowad
Copy link
Contributor

Sure, that's cool.

@bbatsov bbatsov closed this as completed in ead2c5e Mar 5, 2016
@agross
Copy link
Author

agross commented Mar 5, 2016

Thank you!

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

No branches or pull requests

3 participants