Inconsistency between date_select and datetime_select (patch proposal)

[sorry for posting this twice, but i can’t see the first post on the list…]
Help needed:

I’m working on a patch to datetime_select that covers two inconsistencies compared to date_select:

  • should support :order
  • should support :discard_year

I have basically copied and worked with code from date_select and copied the relevant down to datetime_select. But in my opinion, it’s not as beautiful as it could be. So, I’m posting here to get more eyes on it before submitting as a patch.

Any feedback appreciated.

Background on why we should eliminate these inconsitencies in rails core:
http://groups.google.com/group/rubyonrails-core/browse_frm/thread/346cda3cd39e6fa9

/Jesper

File: actionpack/lib/helpers/date_helper.rb:310 (trunk version 5212)

=========changed from:==================
def to_datetime_select_tag(options = {})
defaults = { :discard_type => true }

    options  = defaults.merge(options)
    options_with_prefix = Proc.new { |position|

options.merge(:prefix =>
“#{@object_name}[#{@method_name}(#{position}i)]”) }
value = value(object)

    datetime = options[:include_blank] ? (value || nil) : (value ||

Time.now)

    datetime_select  = select_year(datetime, options_with_prefix.call(1))
    datetime_select << select_month(datetime, options_with_prefix.call(2)) unless options[:discard_month]

    datetime_select << select_day(datetime, options_with_prefix.call(3)) unless options[:discard_day] || options[:discard_month]
    datetime_select << ' &mdash; ' + select_hour(datetime, options_with_prefix.call(4)) unless options[:discard_hour]

    datetime_select << ' : ' + select_minute(datetime, options_with_prefix.call(5)) unless options[:discard_minute] || options[:discard_hour]

    datetime_select
  end
end

=========suggest changed to:=========

  def to_datetime_select_tag(options = {})
    defaults = { :discard_type => true }
    options  = defaults.merge(options)
    options_with_prefix = Proc.new { |position|

options.merge(:prefix => “#{@object_name}[#{@method_name}(#{position}i)]”) }

    datetime = options[:include_blank] ? (value || nil) : (value || Time.now)
    datetime_select = ''
    options[:order] ||= [:year, :month, :day]

    position = {:year => 1, :month => 2, :day => 3}

    discard = {}
    discard[:year]  = true if options[:discard_year]
    discard[:month] = true if options[:discard_month]
    discard[:day]   = true if options[:discard_day] or options[:discard_month]

    options[:order].each do |param|
      datetime_select << self.send("select_#{param}", datetime, options_with_prefix.call(position[param])) unless discard[param]
    end
    datetime_select << " &mdash; " + select_hour(datetime, options_with_prefix.call(4)) unless options[:discard_hour]

    datetime_select << " : "       + select_minute(datetime, options_with_prefix.call(5)) unless options[:discard_minute] || options[:discard_hour]

    datetime_select
  end
end