From 3dba35c07e6ae8fcc2c30abf28c27497540f5fe4 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Mon, 21 Jul 2008 01:36:43 +0200
Subject: [PATCH] Removed unnecessary method DateHelper#leading_zero_on_single_digits and replaced it with sprintf("%02d", number).

---
 actionpack/lib/action_view/helpers/date_helper.rb |   16 ++----
 actionpack/test/template/date_helper_test.rb      |   56 ++++++++++----------
 2 files changed, 34 insertions(+), 38 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb
index 2cdb9a2..20c9153 100755
--- a/actionpack/lib/action_view/helpers/date_helper.rb
+++ b/actionpack/lib/action_view/helpers/date_helper.rb
@@ -359,8 +359,8 @@ module ActionView
           second_options = []
           0.upto(59) do |second|
             second_options << ((val == second) ?
-              content_tag(:option, leading_zero_on_single_digits(second), :value => leading_zero_on_single_digits(second), :selected => "selected") :
-              content_tag(:option, leading_zero_on_single_digits(second), :value => leading_zero_on_single_digits(second))
+              content_tag(:option, sprintf("%02d", second), :value => sprintf("%02d", second), :selected => "selected") :
+              content_tag(:option, sprintf("%02d", second), :value => sprintf("%02d", second))
             )
             second_options << "\n"
           end
@@ -394,8 +394,8 @@ module ActionView
           minute_options = []
           0.step(59, options[:minute_step] || 1) do |minute|
             minute_options << ((val == minute) ?
-              content_tag(:option, leading_zero_on_single_digits(minute), :value => leading_zero_on_single_digits(minute), :selected => "selected") :
-              content_tag(:option, leading_zero_on_single_digits(minute), :value => leading_zero_on_single_digits(minute))
+              content_tag(:option, sprintf("%02d", minute), :value => sprintf("%02d", minute), :selected => "selected") :
+              content_tag(:option, sprintf("%02d", minute), :value => sprintf("%02d", minute))
             )
             minute_options << "\n"
           end
@@ -428,8 +428,8 @@ module ActionView
           hour_options = []
           0.upto(23) do |hour|
             hour_options << ((val == hour) ?
-              content_tag(:option, leading_zero_on_single_digits(hour), :value => leading_zero_on_single_digits(hour), :selected => "selected") :
-              content_tag(:option, leading_zero_on_single_digits(hour), :value => leading_zero_on_single_digits(hour))
+              content_tag(:option, sprintf("%02d", hour), :value => sprintf("%02d", hour), :selected => "selected") :
+              content_tag(:option, sprintf("%02d", hour), :value => sprintf("%02d", hour))
             )
             hour_options << "\n"
           end
@@ -612,10 +612,6 @@ module ActionView
           options[:name] = (options[:prefix] || DEFAULT_PREFIX) + (options[:discard_type] ? '' : "[#{type}]")
           options[:id] = options[:name].gsub(/([\[\(])|(\]\[)/, '_').gsub(/[\]\)]/, '')
         end
-
-        def leading_zero_on_single_digits(number)
-          number > 9 ? number : "0#{number}"
-        end
     end
 
     class InstanceTag #:nodoc:
diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb
index 8b4e94c..0ccd84d 100755
--- a/actionpack/test/template/date_helper_test.rb
+++ b/actionpack/test/template/date_helper_test.rb
@@ -1188,11 +1188,11 @@ class DateHelperTest < ActionView::TestCase
     expected << %{<input type="hidden" id="post_written_on_3i" name="post[written_on(3i)]" value="15" />\n}
 
     expected << %(<select id="post_written_on_4i" name="post[written_on(4i)]">\n)
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %(<select id="post_written_on_5i" name="post[written_on(5i)]">\n)
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, time_select("post", "written_on")
@@ -1203,11 +1203,11 @@ class DateHelperTest < ActionView::TestCase
     @post.written_on = Time.local(2004, 6, 15, 15, 16, 35)
 
     expected = %(<select id="post_written_on_4i" name="post[written_on(4i)]">\n)
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %(<select id="post_written_on_5i" name="post[written_on(5i)]">\n)
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, time_select("post", "written_on", :ignore_date => true)
@@ -1222,15 +1222,15 @@ class DateHelperTest < ActionView::TestCase
     expected << %{<input type="hidden" id="post_written_on_3i" name="post[written_on(3i)]" value="15" />\n}
 
     expected << %(<select id="post_written_on_4i" name="post[written_on(4i)]">\n)
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %(<select id="post_written_on_5i" name="post[written_on(5i)]">\n)
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %(<select id="post_written_on_6i" name="post[written_on(6i)]">\n)
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 35}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 35}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, time_select("post", "written_on", :include_seconds => true)
@@ -1245,11 +1245,11 @@ class DateHelperTest < ActionView::TestCase
     expected << %{<input type="hidden" id="post_written_on_3i" name="post[written_on(3i)]" value="15" />\n}
 
     expected << %(<select id="post_written_on_4i" name="post[written_on(4i)]" class="selector">\n)
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %(<select id="post_written_on_5i" name="post[written_on(5i)]" class="selector">\n)
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, time_select("post", "written_on", {}, :class => 'selector')
@@ -1268,11 +1268,11 @@ class DateHelperTest < ActionView::TestCase
     expected << %{<input type="hidden" id="post_written_on_3i" name="post[written_on(3i)]" value="15" />\n}
 
     expected << %(<select id="post_written_on_4i" name="post[written_on(4i)]" class="selector">\n)
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %(<select id="post_written_on_5i" name="post[written_on(5i)]" class="selector">\n)
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, output_buffer
@@ -1530,15 +1530,15 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_6i" name="post[updated_at(6i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 35}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 35}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :include_seconds => true)
@@ -1559,11 +1559,11 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :discard_year => true)
@@ -1582,11 +1582,11 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :discard_month => true)
@@ -1601,11 +1601,11 @@ class DateHelperTest < ActionView::TestCase
     expected << %{<input type="hidden" id="post_updated_at_3i" name="post[updated_at(3i)]" value="15" />\n}
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :discard_year => true, :discard_month => true)
@@ -1628,11 +1628,11 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :order => [:minute, :day, :hour, :month, :year, :second])
@@ -1653,11 +1653,11 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :order => [:day, :month])
@@ -1680,11 +1680,11 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 15}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 15}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 16}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 16}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :default => Time.local(2006, 9, 19, 15, 16, 35))
@@ -1727,11 +1727,11 @@ class DateHelperTest < ActionView::TestCase
     expected << " &mdash; "
 
     expected << %{<select id="post_updated_at_4i" name="post[updated_at(4i)]">\n}
-    0.upto(23) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 9}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(23) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 9}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
     expected << " : "
     expected << %{<select id="post_updated_at_5i" name="post[updated_at(5i)]">\n}
-    0.upto(59) { |i| expected << %(<option value="#{leading_zero_on_single_digits(i)}"#{' selected="selected"' if i == 42}>#{leading_zero_on_single_digits(i)}</option>\n) }
+    0.upto(59) { |i| expected << %(<option value="#{sprintf("%02d", i)}"#{' selected="selected"' if i == 42}>#{sprintf("%02d", i)}</option>\n) }
     expected << "</select>\n"
 
     assert_dom_equal expected, datetime_select("post", "updated_at", :default => { :month => 10, :minute => 42, :hour => 9 })
-- 
1.5.2.4


From 680409d286c8eadc3a97c13745ba641dd1b05cc5 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Mon, 21 Jul 2008 01:43:40 +0200
Subject: [PATCH] DRYed up DateHelper methods select_second, select_minute, select_hour,
select_day and select_year by moving creation of <option> tags into
build_options method. Not used by select_month because it would
unnecessarily bloat the build_options method.
---
 actionpack/lib/action_view/helpers/date_helper.rb |  105 +++++++--------------
 1 files changed, 34 insertions(+), 71 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb
index 20c9153..613fb6a 100755
--- a/actionpack/lib/action_view/helpers/date_helper.rb
+++ b/actionpack/lib/action_view/helpers/date_helper.rb
@@ -353,19 +353,8 @@ module ActionView
       #
       def select_second(datetime, options = {}, html_options = {})
         val = datetime ? (datetime.kind_of?(Fixnum) ? datetime : datetime.sec) : ''
-        if options[:use_hidden]
-          options[:include_seconds] ? hidden_html(options[:field_name] || 'second', val, options) : ''
-        else
-          second_options = []
-          0.upto(59) do |second|
-            second_options << ((val == second) ?
-              content_tag(:option, sprintf("%02d", second), :value => sprintf("%02d", second), :selected => "selected") :
-              content_tag(:option, sprintf("%02d", second), :value => sprintf("%02d", second))
-            )
-            second_options << "\n"
-          end
-          select_html(options[:field_name] || 'second', second_options.join, options, html_options)
-        end
+        options[:use_hidden] ? (options[:include_seconds] ? hidden_html(options[:field_name] || 'second', val, options) : '') :
+          select_html(options[:field_name] || 'second', build_options(val), options, html_options)
       end
 
       # Returns a select tag with options for each of the minutes 0 through 59 with the current minute selected.
@@ -388,19 +377,8 @@ module ActionView
       #
       def select_minute(datetime, options = {}, html_options = {})
         val = datetime ? (datetime.kind_of?(Fixnum) ? datetime : datetime.min) : ''
-        if options[:use_hidden]
-          hidden_html(options[:field_name] || 'minute', val, options)
-        else
-          minute_options = []
-          0.step(59, options[:minute_step] || 1) do |minute|
-            minute_options << ((val == minute) ?
-              content_tag(:option, sprintf("%02d", minute), :value => sprintf("%02d", minute), :selected => "selected") :
-              content_tag(:option, sprintf("%02d", minute), :value => sprintf("%02d", minute))
-            )
-            minute_options << "\n"
-          end
-          select_html(options[:field_name] || 'minute', minute_options.join, options, html_options)
-         end
+        options[:use_hidden] ? hidden_html(options[:field_name] || 'minute', val, options) :
+          select_html(options[:field_name] || 'minute', build_options(val, :step => options[:minute_step]), options, html_options)
       end
 
       # Returns a select tag with options for each of the hours 0 through 23 with the current hour selected.
@@ -422,19 +400,8 @@ module ActionView
       #
       def select_hour(datetime, options = {}, html_options = {})
         val = datetime ? (datetime.kind_of?(Fixnum) ? datetime : datetime.hour) : ''
-        if options[:use_hidden]
-          hidden_html(options[:field_name] || 'hour', val, options)
-        else
-          hour_options = []
-          0.upto(23) do |hour|
-            hour_options << ((val == hour) ?
-              content_tag(:option, sprintf("%02d", hour), :value => sprintf("%02d", hour), :selected => "selected") :
-              content_tag(:option, sprintf("%02d", hour), :value => sprintf("%02d", hour))
-            )
-            hour_options << "\n"
-          end
-          select_html(options[:field_name] || 'hour', hour_options.join, options, html_options)
-        end
+        options[:use_hidden] ? hidden_html(options[:field_name] || 'hour', val, options) : 
+          select_html(options[:field_name] || 'hour', build_options(val, :end => 23), options, html_options)
       end
 
       # Returns a select tag with options for each of the days 1 through 31 with the current day selected.
@@ -456,19 +423,9 @@ module ActionView
       #
       def select_day(date, options = {}, html_options = {})
         val = date ? (date.kind_of?(Fixnum) ? date : date.day) : ''
-        if options[:use_hidden]
-          hidden_html(options[:field_name] || 'day', val, options)
-        else
-          day_options = []
-          1.upto(31) do |day|
-            day_options << ((val == day) ?
-              content_tag(:option, day, :value => day, :selected => "selected") :
-              content_tag(:option, day, :value => day)
-            )
-            day_options << "\n"
-          end
-          select_html(options[:field_name] || 'day', day_options.join, options, html_options)
-        end
+        options[:use_hidden] ?  hidden_html(options[:field_name] || 'day', val, options) :
+          select_html(options[:field_name] || 'day', build_options(val, :start => 1, :end => 31, :leading_zeros => false), 
+                      options, html_options)
       end
 
       # Returns a select tag with options for each of the months January through December with the current month selected.
@@ -562,35 +519,41 @@ module ActionView
       #
       def select_year(date, options = {}, html_options = {})
         if !date || date == 0
-          value = ''
+          val = ''
           middle_year = Date.today.year
         elsif date.kind_of?(Fixnum)
-          value = middle_year = date
+          val = middle_year = date
         else
-          value = middle_year = date.year
+          val = middle_year = date.year
         end
-
+ 
         if options[:use_hidden]
-          hidden_html(options[:field_name] || 'year', value, options)
+          hidden_html(options[:field_name] || 'year', val, options)
         else
-          year_options = ''
-          start_year   = options[:start_year] || middle_year - 5
-          end_year     = options[:end_year]   || middle_year + 5
-          step_val     = start_year < end_year ? 1 : -1
-
-          start_year.step(end_year, step_val) do |year|
-            if value == year
-              year_options << content_tag(:option, year, :value => year, :selected => "selected")
-            else
-              year_options << content_tag(:option, year, :value => year)
-            end
-            year_options << "\n"
-          end
-          select_html(options[:field_name] || 'year', year_options, options, html_options)
+          options[:start_year] ||= middle_year - 5
+          options[:end_year]   ||= middle_year + 5
+          step                   = options[:start_year] < options[:end_year] ? 1 : -1
+          
+          select_html(options[:field_name] || 'year', build_options(val, :start => options[:start_year], :end => options[:end_year], 
+                                                                         :step => step, :leading_zeros => false), options, html_options)
         end
       end
 
       private
+      
+        def build_options(selected, options={})
+          options.reverse_merge!(:start => 0, :end => 59, :step => 1, :leading_zeros => true)
+        
+          select_options = []
+          (options[:start] || 0).step((options[:end] || 59), options[:step] || 1) do |i|
+            value = options[:leading_zeros] ? sprintf("%02d", i) : i
+            tag_options = { :value => value }
+            tag_options[:selected] = "selected" if selected == i
+          
+            select_options << content_tag(:option, value, tag_options)
+          end
+          select_options.join("\n") + "\n"
+        end
 
         def select_html(type, html_options, options, select_tag_options = {})
           name_and_id_from_options(options, type)
-- 
1.5.2.4


From 02f8fede544563c600cff2fb533950c9864fbf28 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Mon, 21 Jul 2008 01:51:41 +0200
Subject: [PATCH] Introduced option :date_separator for DateHelper methods select_date and select_datetime. DRYed up code a little in select_date. Added tests and documentation.

---
 actionpack/lib/action_view/helpers/date_helper.rb |   24 ++++++++++------
 actionpack/test/template/date_helper_test.rb      |   32 +++++++++++++++++++++
 2 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/actionpack/lib/action_view/helpers/date_helper.rb b/actionpack/lib/action_view/helpers/date_helper.rb
index 613fb6a..d89ed62 100755
--- a/actionpack/lib/action_view/helpers/date_helper.rb
+++ b/actionpack/lib/action_view/helpers/date_helper.rb
@@ -227,9 +227,10 @@ module ActionView
 
       # Returns a set of html select-tags (one for year, month, day, hour, and minute) pre-selected with the +datetime+.
       # It's also possible to explicitly set the order of the tags using the <tt>:order</tt> option with an array of
-      # symbols <tt>:year</tt>, <tt>:month</tt> and <tt>:day</tt> in the desired order. If you do not supply a Symbol, it
-      # will be appended onto the <tt>:order</tt> passed in. You can also add <tt>:date_separator</tt> and <tt>:time_separator</tt>
-      # keys to the +options+ to control visual display of the elements.
+      # symbols <tt>:year</tt>, <tt>:month</tt> and <tt>:day</tt> in the desired order. If you do not supply a Symbol, 
+      # it will be appended onto the <tt>:order</tt> passed in. You can also add <tt>:date_separator</tt>,
+      # <tt>:datetime_separator</tt> and <tt>:time_separator</tt> keys to the +options+ to control visual display of 
+      # the elements.
       #
       # If anything is passed in the html_options hash it will be applied to every select tag in the set.
       #
@@ -250,6 +251,11 @@ module ActionView
       #   # with a '/' between each date field.
       #   select_datetime(my_date_time, :date_separator => '/')
       #
+      #   # Generates a datetime select that defaults to the datetime in my_date_time (four days after today)
+      #   # with a date fields separated by '/', time fields separated by '' and the date and time fields
+      #   # separated by a comma (',').
+      #   select_datetime(my_date_time, :date_separator => '/', :time_separator => '', :datetime_separator => ',')
+      #
       #   # Generates a datetime select that discards the type of the field and defaults to the datetime in 
       #   # my_date_time (four days after today)
       #   select_datetime(my_date_time, :discard_type => true)
@@ -287,19 +293,19 @@ module ActionView
       #   # my_date (six days after today)
       #   select_date(my_date, :discard_type => true)
       #
+      #   # Generates a date select that defaults to the date in my_date,
+      #   # which has fields separated by '/' 
+      #   select_date(my_date, :date_separator => '/')
+      #
       #   # Generates a date select that defaults to the datetime in my_date (six days after today)
       #   # prefixed with 'payday' rather than 'date'
       #   select_date(my_date, :prefix => 'payday')
       #
       def select_date(date = Date.current, options = {}, html_options = {})
-        options[:order] ||= []
+        options.reverse_merge!(:order => [], :date_separator => '')
         [:year, :month, :day].each { |o| options[:order].push(o) unless options[:order].include?(o) }
 
-        select_date = ''
-        options[:order].each do |o|
-          select_date << self.send("select_#{o}", date, options, html_options)
-        end
-        select_date
+        options[:order].inject([]) { |s, o| s << self.send("select_#{o}", date, options, html_options) }.join(options[:date_separator])
       end
 
       # Returns a set of html select-tags (one for hour and minute)
diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb
index 0ccd84d..70a9bf5 100755
--- a/actionpack/test/template/date_helper_test.rb
+++ b/actionpack/test/template/date_helper_test.rb
@@ -856,6 +856,38 @@ class DateHelperTest < ActionView::TestCase
 
     assert_dom_equal expected, select_datetime(Time.mktime(2003, 8, 16, 8, 4, 18), {:start_year => 2003, :end_year => 2005, :prefix => "date[first]"}, :class => 'selector')
   end
+  
+  def test_select_datetime_with_all_separators
+    expected =  %(<select id="date_first_year" name="date[first][year]" class="selector">\n)
+    expected << %(<option value="2003" selected="selected">2003</option>\n<option value="2004">2004</option>\n<option value="2005">2005</option>\n)
+    expected << "</select>\n"
+    
+    expected << "/"
+ 
+    expected << %(<select id="date_first_month" name="date[first][month]" class="selector">\n)
+    expected << %(<option value="1">January</option>\n<option value="2">February</option>\n<option value="3">March</option>\n<option value="4">April</option>\n<option value="5">May</option>\n<option value="6">June</option>\n<option value="7">July</option>\n<option value="8" selected="selected">August</option>\n<option value="9">September</option>\n<option value="10">October</option>\n<option value="11">November</option>\n<option value="12">December</option>\n)
+    expected << "</select>\n"
+    
+    expected << "/"
+ 
+    expected << %(<select id="date_first_day" name="date[first][day]" class="selector">\n)
+    expected << %(<option value="1">1</option>\n<option value="2">2</option>\n<option value="3">3</option>\n<option value="4">4</option>\n<option value="5">5</option>\n<option value="6">6</option>\n<option value="7">7</option>\n<option value="8">8</option>\n<option value="9">9</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16" selected="selected">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n)
+    expected << "</select>\n"
+    
+    expected << "&mdash;"
+ 
+    expected << %(<select id="date_first_hour" name="date[first][hour]" class="selector">\n)
+    expected << %(<option value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08" selected="selected">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n)
+    expected << "</select>\n"
+    
+    expected << ":"
+ 
+    expected << %(<select id="date_first_minute" name="date[first][minute]" class="selector">\n)
+    expected << %(<option value="00">00</option>\n<option value="01">01</option>\n<option value="02">02</option>\n<option value="03">03</option>\n<option value="04" selected="selected">04</option>\n<option value="05">05</option>\n<option value="06">06</option>\n<option value="07">07</option>\n<option value="08">08</option>\n<option value="09">09</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n<option value="32">32</option>\n<option value="33">33</option>\n<option value="34">34</option>\n<option value="35">35</option>\n<option value="36">36</option>\n<option value="37">37</option>\n<option value="38">38</option>\n<option value="39">39</option>\n<option value="40">40</option>\n<option value="41">41</option>\n<option value="42">42</option>\n<option value="43">43</option>\n<option value="44">44</option>\n<option value="45">45</option>\n<option value="46">46</option>\n<option value="47">47</option>\n<option value="48">48</option>\n<option value="49">49</option>\n<option value="50">50</option>\n<option value="51">51</option>\n<option value="52">52</option>\n<option value="53">53</option>\n<option value="54">54</option>\n<option value="55">55</option>\n<option value="56">56</option>\n<option value="57">57</option>\n<option value="58">58</option>\n<option value="59">59</option>\n)
+    expected << "</select>\n"
+ 
+    assert_dom_equal expected, select_datetime(Time.mktime(2003, 8, 16, 8, 4, 18), { :datetime_separator => "&mdash;", :date_separator => "/", :time_separator => ":", :start_year => 2003, :end_year => 2005, :prefix => "date[first]"}, :class => 'selector')
+  end
 
   def test_select_time
     expected = %(<select id="date_hour" name="date[hour]">\n)
-- 
1.5.2.4


From 598f89ed323c88cd83872da38d0eca520bc12ed5 Mon Sep 17 00:00:00 2001
From: Clemens Kofler <clemens@railway.at>
Date: Mon, 21 Jul 2008 01:59:34 +0200
Subject: [PATCH] Added missing test for DateHelper#select_date to verify functionality for :date_separator option.

---
 actionpack/test/template/date_helper_test.rb |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/actionpack/test/template/date_helper_test.rb b/actionpack/test/template/date_helper_test.rb
index 70a9bf5..d50243d 100755
--- a/actionpack/test/template/date_helper_test.rb
+++ b/actionpack/test/template/date_helper_test.rb
@@ -756,6 +756,26 @@ class DateHelperTest < ActionView::TestCase
 
     assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), {:start_year => 2003, :end_year => 2005, :prefix => "date[first]"}, :class => "selector")
   end
+  
+  def test_select_date_with_separator
+    expected =  %(<select id="date_first_year" name="date[first][year]">\n)
+    expected << %(<option value="2003" selected="selected">2003</option>\n<option value="2004">2004</option>\n<option value="2005">2005</option>\n)
+    expected << "</select>\n"
+    
+    expected << " / "
+
+    expected << %(<select id="date_first_month" name="date[first][month]">\n)
+    expected << %(<option value="1">January</option>\n<option value="2">February</option>\n<option value="3">March</option>\n<option value="4">April</option>\n<option value="5">May</option>\n<option value="6">June</option>\n<option value="7">July</option>\n<option value="8" selected="selected">August</option>\n<option value="9">September</option>\n<option value="10">October</option>\n<option value="11">November</option>\n<option value="12">December</option>\n)
+    expected << "</select>\n"
+    
+    expected << " / "
+
+    expected << %(<select id="date_first_day" name="date[first][day]">\n)
+    expected << %(<option value="1">1</option>\n<option value="2">2</option>\n<option value="3">3</option>\n<option value="4">4</option>\n<option value="5">5</option>\n<option value="6">6</option>\n<option value="7">7</option>\n<option value="8">8</option>\n<option value="9">9</option>\n<option value="10">10</option>\n<option value="11">11</option>\n<option value="12">12</option>\n<option value="13">13</option>\n<option value="14">14</option>\n<option value="15">15</option>\n<option value="16" selected="selected">16</option>\n<option value="17">17</option>\n<option value="18">18</option>\n<option value="19">19</option>\n<option value="20">20</option>\n<option value="21">21</option>\n<option value="22">22</option>\n<option value="23">23</option>\n<option value="24">24</option>\n<option value="25">25</option>\n<option value="26">26</option>\n<option value="27">27</option>\n<option value="28">28</option>\n<option value="29">29</option>\n<option value="30">30</option>\n<option value="31">31</option>\n)
+    expected << "</select>\n"
+
+    assert_dom_equal expected, select_date(Time.mktime(2003, 8, 16), { :date_separator => " / ", :start_year => 2003, :end_year => 2005, :prefix => "date[first]"})
+  end
 
   def test_select_datetime
     expected =  %(<select id="date_first_year" name="date[first][year]">\n)
-- 
1.5.2.4