wbailey / command_line_reporter

A gem for making it easy to produce a report while a ruby script is executing
Apache License 2.0
432 stars 23 forks source link

fix inheritance when first row is header #11

Closed selu closed 10 years ago

selu commented 10 years ago

Error message when first row is header:

 ../gems/command_line_reporter-3.3.1/lib/command_line_reporter/table.rb:118:in `use_color': undefined method `columns' for nil:NilClass (NoMethodError)

All tests were green, thus spec/table_spec.rb was checked and fixed first:

The following condition allow access 'inherit_from' row if it exists

if self.rows.size > inherit_from
wbailey commented 10 years ago

Hi @selu! Thanks for the pull request! I think this pull request doesn't preserve the essence of the original test. The point of the inheritance model is to allow the second row to be bold regardless of whether there is a header row or not because it is defined at the row level.

Can you share the code that produced the error above? I am not understanding how the error you started off in the pull request is reflected in the tests properly. It also seems there are some issues according to ruport with your pull request as found by @houndci

selu commented 10 years ago

Hi, you can try this short example, it's based on your test:

require 'command_line_reporter'

@table = CommandLineReporter::Table.new
row = CommandLineReporter::Row.new(:header => true)
(
  @cols1 = [
    CommandLineReporter::Column.new('asdf', :width => 5),
    CommandLineReporter::Column.new('qwer', :align => 'right', :color => 'purple'),
    CommandLineReporter::Column.new('tutu', :color => 'green'),
    CommandLineReporter::Column.new('uiui', :bold => true),
  ]
).each {|c| row.add(c)}
@table.add(row)
row = CommandLineReporter::Row.new
(@cols2 = [
  CommandLineReporter::Column.new('test'),
  CommandLineReporter::Column.new('test'),
  CommandLineReporter::Column.new('test', :color => 'blue'),
  CommandLineReporter::Column.new('test'),
]
).each {|c| row.add(c)}
@table.add(row)

Your test in table_spec.rb does not fail because a table is built in the before filter, what changes Column objects @cols1 in @cols2, that's why I reorganized the test first.

Yes, I realized issues by @houndci, actually most of them were realized before I sent my pull request, just I kept the same style in my changes what's used in the given files already. If you want, I can rework my request to pass @houndci