tpope/fivemat — pull request #21 by myronmarston (2014-05-15 07:19:44+00:00)
Title: Update rspec 3 formatter to work with latest rspec 3 changes.

@xaviershay / @JonRowe -- I took a stab at updating fivemat to work with the latest changes in rspec/rspec-core#1519 and this is what I came up with. For now I just made a PR against my fork of Fivemat so we have a forum for discussion since didn't want to subject tpope to the noise while we're wrapping this up. With my changes here it no longer subclasses one of RSpec's formatter classes. Note that it's currently more code than before, and I think that's a signal we've got further to go. Having 3rd party formatters use delegation instead of inheritance primarily benefits us -- it makes it easier for us to refactor/change RSpec's internals w/o worrying about the brittle base class problem. If it's still easier for end-users to write a custom formatter using subclassing they'll probably still do it and I wouldn't blame them.

A few things I noticed:

* `dump_pending_fixed` was registered as a notification but actually isn't one. I opened rspec/rspec-core#1530 to suggest we surface this type of issue somehow.
* `seed` is a separate notification form `dump_summary`. I think they should be combined...or at least, the notification object provided to `dump_summary` should provide a method that provides the full typical summary output including the seed. Otherwise I think it's too easy to forget (or not realize the need) as @xaviershay did before...and then the seed isn't printed at all which is problematic.
* IMO it's too much effort here in `dump_summary` and `dump_failure`. I copied code over from rspec-core (including the `short_padding` and `long_padding` methods). Users shouldn't have to do that. I think we should provide methods that provide the full default output for each notification event (in addition to the individual pieces we provide now). This allows formatters that want to be very particular about some things to do so while still making it easy to emit the same format as rspec-core (even as rspec-core evolves and starts to include more info) w/ minimal effort.
* Jon and I discussed this a bit in rspec/rspec-core#1530, but I think making the user pass the `ConsoleCodes` arguments to all the methods is really cumbersome, particularly when it's the 99% case. I'd like to see those methods provide `ConsoleCodes` as a default argument value so that the colorizer can be overriden but in the common case it doesn't require so much effort.
* Outputting in color in general is too much effort. I wrote the `color` helper method to help with that, but it feels like a hack that I used `public_send(:"#{color_type}_color}")` there to get the configured color code. We should expose a method that already does this.

All that said -- I'm really itching to release RSpec 3 and don't want to delay it for weeks while we figure this out. So, my suggestion is to figure out what if any further improvements require breaking changes (hopefully none...) and make those, and then do the rest of this as additive "new features" in 3.1 (and plan on having that ship not too long after 3.0 -- maybe even a few weeks after).

Thoughts?