awesome-print / awesome_print

Pretty print your Ruby objects with style -- in full color and with proper indentation
http://github.com/michaeldv/awesome_print
MIT License
4.08k stars 454 forks source link

Printing of ENV breaks other things #134

Closed romanbsd closed 9 years ago

romanbsd commented 11 years ago

This commit broke the printing of objects which have IPAddr: 6a820e1f730 The IPAddr#==(other) tries to convert ENV to IPAddr for comparison and fails. Maybe it's better just to has if respond_to?(:to_hash) there.

michaeldv commented 11 years ago

This looks like ipaddr bug in MRI. Try this (awesome_print is not loaded):

$ irb
irb(main):001:0> require "ipaddr"
=> true
irb(main):002:0> ip = IPAddr.new("3ffe:505:2::1")
=> #<IPAddr: IPv6:3ffe:0505:0002:0000:0000:0000:0000:0001/ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff>
irb(main):003:0> ip == ENV
NoMethodError: undefined method `to_i' for #<Object:0x007f90eb8d1750>
    from ~/.rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/ipaddr.rb:471:in `initialize'
    from ~/.rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/ipaddr.rb:516:in `new'
    from ~/.rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/ipaddr.rb:516:in `coerce_other'
    from ~/.rubies/ruby-2.0.0-p247/lib/ruby/2.0.0/ipaddr.rb:150:in `=='
    from (irb):3
    from ~/.rubies/ruby-2.0.0-p247/bin/irb:12:in `<main>'
irb(main):004:0>
romanbsd commented 11 years ago

Agree, but it renders awesome print useless. What about checking for prsence of to_hash?

michaeldv commented 11 years ago

Well, the question is whether to use what you're proposing:

if object.respond_to?(:to_hash)
  awesome_hash(object.to_hash)

versus implementing specific workaround:

if object.eql?(ENV)
  awesome_hash(ENV.to_hash)
romanbsd commented 11 years ago

Your take. I think that object.respond_to?(:to_hash) is a better approach, but on the other hand, I wouldn't change behavior in minor version (semantic versioning), so object.eql?(ENV) is a better candidate.

michaeldv commented 11 years ago

You were right, see https://github.com/michaeldv/awesome_print/issues/139 ;-)