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

Add a NullFormatter class #3

Closed ghost closed 12 years ago

ghost commented 12 years ago

I added a new formatter, NullFormatter. This formatter eats all its output.

The motivation for this is that I have a bunch of existing code that uses many puts statements for output, but which respects a 'quiet' flag that can be passed to suppress the output. With this new formatter, I can start to transition the existing code to command_line_reporter by initializing this way:

quiet? ? self.formatter = 'null' : self.formatter = 'nested'

This does end up being a pretty substantial change, because to make it work all output has to be sent through the formatters - no more raw puts statements in the top-level class. But I think it's a good one, and it potentially opens the opportunity for a more flexible interpretation of formatters in the future - an EmailFormatter for example.

The second commit here just fixed a trivial typo in an example comment. You probably want that one even if you'd like to pass on the null formatter ;)

wbailey commented 12 years ago

Thanks for the typo catch and I love the idea but I think there is a simpler solution to support your --quiet flag that only requires a couple lines of changes to your scripts without change CLR:

require 'command_line_reporter'
require 'stringio'

class Example
  include CommandLineReporter

  def initialize
    self.formatter = 'progress'
    $stdout = StringIO.new if ENV['quiet']
  end

  def run
    x = 0

    report do
      10.times do
        x += 1
        sleep 0.1
        formatter.progress
      end
    end
  ensure
    $stdout = STDOUT
  end
end

print 'Losing connection'

Example.new.run

puts "...and we're back"

If we run it with the env var defined then we get:

[~/work/command_line_reporter]$ export quiet=1
[~/work/command_line_reporter]$ ruby -I lib examples/quiet.rb
Losing connection...and we're back

So this swallows your output and I think leaves everything else intact. Thoughts?

ghost commented 12 years ago

I thought about that, but that removes the possibility of keeping some output while suppressing the progress bits. So it doesn't quite support my own needs. But it's the 80% case, I expect, so if you'd like to keep things simpler, it wouldn't bother me.

wbailey commented 12 years ago

A compromise that would work would be adding 2 new methods to the mixin interface:

suppress_output restrore_output

That would provide a lot of granularity and handle your example I believe like this example:


require 'command_line_reporter'

class Example
  include CommandLineReporter

  def initialize
    self.formatter = 'progress'
  end

  def run
    x = 0

    suppress_output if ENV['quiet']

    report do
      10.times do
        x += 1
        sleep 0.1
        formatter.progress
      end
    end

    restore_output
  end
end

print 'Losing connection'

Example.new.run

puts "...and we're back"

which produces the same output as the previous example.

ghost commented 12 years ago

That'd work. I'll be happy to take a run at that (in my copious spare time ) if you want, though I think you've already written most of it.

Stepping back a bit, I'm still not 100% happy from an engineering perspective about the way that output is scattered around classes in the current code. But that's your call, not mine.

wbailey commented 12 years ago

I already added the methods to the api since it was all of 6 lines. You do have my interest piqued on the class design. I assume you are referring to how the output of tables is produced? Curious to hear your thoughts on a better overall design.

ghost commented 12 years ago

I think it comes down to whether the job of formatter is just to override selectively, or to take care of all output. Feels to me like the formatter should own the whole job, with other classes passing down to it - which was the point of my big changes. But that's somewhat of a hijack of your original ideas. I do suspect in the long run it'd be more maintainable to centralize the output, rather than scatter it around.

No big deal either way though.