Fwd: 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



-- 

-- 
Jesper Rønn-Jensen
http://justaddwater.dk/ (weblog)
--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the Google Groups "Ruby on Rails: Core" group. 
 To post to this group, send email to rubyonrails-core@xxxxxxxxxxxxxxxx 
 To unsubscribe from this group, send email to rubyonrails-core-unsubscribe@xxxxxxxxxxxxxxxx 
 For more options, visit this group at http://groups.google.com/group/rubyonrails-core 
 -~----------~----~----~----~------~----~------~--~---