cldwalker / hirb

A mini view framework for console/irb that's easy to use, even while under its influence. Console goodies include a no-wrap table, auto-pager, tree and menu.
http://tagaholic.me/hirb/
MIT License
1.65k stars 72 forks source link

[Bug] Paginate objects obtained by polymorphic association #18

Closed bogdan closed 14 years ago

bogdan commented 14 years ago

Let's say that we have a polymorphic association: class Order belongs_to :orderable, :polymorphic => true end In this case hirb will try to paginate the following query: Order.all.map(&:orderable) That cause bugs because the result contains instances of different models that have different columns.

I don't know if hirb can help display these objects somehow but anyway it should not raise exceptions.

cldwalker commented 14 years ago

Hirb doesn't paginate ActiveRecord objects. Could you give the specific queries and errors you're getting? I can't help without specifics.

bogdan commented 14 years ago

May be 'paginate' was a wrong word. I mean to say display result with pager.

Here is the error
Hirb Error: undefined method title' for #<Album:0xb6535c30> /home/bogdan/makabu/railsware/yawma/repository/vendor/rails/activerecord/lib/active_record/attribute_methods.rb:260:inmethod_missing' /home/bogdan/makabu/railsware/yawma/repository/vendor/rails/activerecord/lib/active_record/associations/association_proxy.rb:148:in send' /home/bogdan/makabu/railsware/yawma/repository/vendor/rails/activerecord/lib/active_record/associations/association_proxy.rb:148:insend' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:10:in render' /home/bogdan/makabu/railsware/yawma/repository/vendor/rails/activerecord/lib/active_record/attribute_methods.rb:211:ininject' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:10:in each' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:10:ininject' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:10:in render' /usr/lib/ruby/1.8/irb/workspace.rb:81:ininject' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:9:in each' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:9:ininject' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/object_table.rb:9:in render' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/helpers/auto_table.rb:22:inrender' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/formatter.rb:64:in _format_output' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/formatter.rb:51:informat_output' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/view.rb:213:in render_output' /var/lib/gems/1.8/gems/hirb-0.3.2/lib/hirb/view.rb:126:inview_output'

In my case orderable could be an instance of Album or Music. Column title exist for music but not for Album.

cldwalker commented 14 years ago

Ok. This error isn't related to hirb's paging but rather its rendering functionality. This issue has been brought up before and I still have no need to add support for it. You're welcome to add support using this post as a guide on creating and using your own helper. Since you want each row to possibly display a different set of columns, you'll need to map an array of activerecord objects to an array of hashes, with keys representing a column and values representing the column values. You'll also want to pass the :all_fields option to super().

bogdan commented 14 years ago

Why not to do this? http://github.com/bogdan/hirb/commit/92af9a976a2309ca0fe27d84ef754e01e4bd12db (rough code just to demonstrate the idea)

cldwalker commented 14 years ago

Well, hirb already rescues rendering exceptions. How else would you see 'Hirb Error: '? What you're suggesting is rendering the irb default if rendering fails which is great for this particular case but not so great for the other 99% of the time when you want to correct faulty helpers.

I'm not interested in this feature but if you want to add it with tests, I'd strongly consider pulling it. To accomplish the behavior you want I would add a configurable option in that rescue i.e.:

rescue Exception=>e
if config[:silence_errors]
    warn e
    false
 else
   # put existing rescue code here
 end

You could then enable your behavior with: Hirb.enable :silence_errors=>true

bogdan commented 14 years ago

Unfortunately 99% of people doesn't know what is Hirb helpers. What do you think about this approach?

  def view_output(output, options={})
    enabled? && config[:formatter] && render_output(output, options)
  rescue Exception=>e
    if config[:debug]
      index = (obj = e.backtrace.find {|f| f =~ /^\(eval\)/}) ? e.backtrace.index(obj) : e.backtrace.length
      $stderr.puts "Hirb Error: #{e.message}", e.backtrace.slice(0,index).map {|e| "    " + e }
    else
      puts output.inspect
    end
    true
  end
cldwalker commented 14 years ago

Sorry but I'm not interested in silencing errors as the default. It's not hard to pass an extra option to Hirb.enable if you really want this behavior. Feel free to implement as I suggested before or I can close the ticket.

bogdan commented 14 years ago

Here is my patch as you suggested before. http://github.com/bogdan/hirb/commit/00e83379d18cee85fef90750f0b8d812366e863d

Ping me if anything is wrong. Thanks.

cldwalker commented 14 years ago

Renamed silence_errors option to ignore_errors. Made some tweaks to original patch. Closed by a80e01072818cff5d73539b8a8acfc5849421909.