Tidied and standardised text_helper implementations.

2 files changed, 62 insertions, 14 deletions

diff --git a/actionpack/lib/action_view/helpers/text_helper.rb b/actionpack/lib/action_view/helpers/text_helper.rb
index da8c0d1de6..54a65ce5f8 100644
--- a/actionpack/lib/action_view/helpers/text_helper.rb
+++ b/actionpack/lib/action_view/helpers/text_helper.rb
@@ -81,8 +81,7 @@ module ActionView
       #   truncate("<p>Once upon a time in a world far far away</p>")
       #   # => "<p>Once upon a time in a wo..."
       def truncate(text, options = {})
-        options.reverse_merge!(:length => 30)
-        text.truncate(options.delete(:length), options) if text
+        text.truncate(options.fetch(:length, 30), options) if text
       end
 
       # Highlights one or more +phrases+ everywhere in +text+ by inserting it into
@@ -102,14 +101,14 @@ module ActionView
       #   highlight('You searched for: rails', 'rails', :highlighter => '<a href="search?q=\1">\1</a>')
       #   # => You searched for: <a href="search?q=rails">rails</a>
       def highlight(text, phrases, options = {})
-        options[:highlighter] ||= '<mark>\1</mark>'
-
-        text = sanitize(text) unless options[:sanitize] == false
+        highlighter = options.fetch(:highlighter, '<mark>\1</mark>')
+        
+        text = sanitize(text) if options.fetch(:sanitize, true)
         if text.blank? || phrases.blank?
           text
         else
           match = Array(phrases).map { |p| Regexp.escape(p) }.join('|')
-          text.gsub(/(#{match})(?![^<]*?>)/i, options[:highlighter])
+          text.gsub(/(#{match})(?![^<]*?>)/i, highlighter)
         end.html_safe
       end
 
@@ -135,8 +134,8 @@ module ActionView
       #   # => <chop> is also an example
       def excerpt(text, phrase, options = {})
         return unless text && phrase
-        radius   = options[:radius]   || 100
-        omission = options[:omission] || "..."
+        radius   = options.fetch(:radius, 100)
+        omission = options.fetch(:omission, "...")
 
         phrase = Regexp.escape(phrase)
         return unless found_pos = text =~ /(#{phrase})/i
@@ -152,7 +151,7 @@ module ActionView
 
       # Attempts to pluralize the +singular+ word unless +count+ is 1. If
       # +plural+ is supplied, it will use that when count is > 1, otherwise
-      # it will use the Inflector to determine the plural form
+      # it will use the Inflector to determine the plural form.
       #
       #   pluralize(1, 'person')
       #   # => 1 person
@@ -166,7 +165,13 @@ module ActionView
       #   pluralize(0, 'person')
       #   # => 0 people
       def pluralize(count, singular, plural = nil)
-        "#{count || 0} " + ((count == 1 || count =~ /^1(\.0+)?$/) ? singular : (plural || singular.pluralize))
+        word = if (count == 1 || count =~ /^1(\.0+)?$/) 
+          singular 
+        else
+          plural || singular.pluralize
+        end
+        
+        "#{count || 0} #{word}"
       end
 
       # Wraps the +text+ into lines no longer than +line_width+ width. This method
@@ -185,7 +190,7 @@ module ActionView
       #   word_wrap('Once upon a time', :line_width => 1)
       #   # => Once\nupon\na\ntime
       def word_wrap(text, options = {})
-        line_width = options[:line_width] || 80
+        line_width = options.fetch(:line_width, 80)
 
         text.split("\n").collect do |line|
           line.length > line_width ? line.gsub(/(.{1,#{line_width}})(\s+|$)/, "\\1\n").strip : line
@@ -221,9 +226,10 @@ module ActionView
       #
       #   simple_format("<span>I'm allowed!</span> It's true.", {}, :sanitize => false)
       #   # => "<p><span>I'm allowed!</span> It's true.</p>"
-      def simple_format(text, html_options={}, options={})
-        text = sanitize(text) unless options[:sanitize] == false
+      def simple_format(text, html_options = {}, options = {})
         wrapper_tag = options.fetch(:wrapper_tag, :p)
+        
+        text = sanitize(text) if options.fetch(:sanitize, true)
         paragraphs = split_paragraphs(text)
 
         if paragraphs.empty?
@@ -274,7 +280,7 @@ module ActionView
       #  <% end %>
       def cycle(first_value, *values)
         options = values.extract_options!
-        name = options.fetch(:name, "default")
+        name = options.fetch(:name, 'default')
 
         values.unshift(first_value)
 
diff --git a/actionpack/test/template/text_helper_test.rb b/actionpack/test/template/text_helper_test.rb
index 66fab20710..f58e474759 100644
--- a/actionpack/test/template/text_helper_test.rb
+++ b/actionpack/test/template/text_helper_test.rb
@@ -60,6 +60,20 @@ class TextHelperTest < ActionView::TestCase
     simple_format(text)
     assert_equal text_clone, text
   end
+  
+  def test_simple_format_does_not_modify_the_html_options_hash
+    options = { :class => "foobar"}
+    passed_options = options.dup
+    simple_format("some text", passed_options)
+    assert_equal options, passed_options
+  end
+  
+  def test_simple_format_does_not_modify_the_options_hash
+    options = { :wrapper_tag => :div, :sanitize => false }
+    passed_options = options.dup
+    simple_format("some text", {}, passed_options)
+    assert_equal options, passed_options
+  end
 
   def test_truncate_should_not_be_html_safe
     assert !truncate("Hello World!", :length => 12).html_safe?
@@ -92,6 +106,13 @@ class TextHelperTest < ActionView::TestCase
     assert_equal "\354\225\204\353\246\254\353\236\221 \354\225\204\353\246\254 ...".force_encoding('UTF-8'),
       truncate("\354\225\204\353\246\254\353\236\221 \354\225\204\353\246\254 \354\225\204\353\235\274\353\246\254\354\230\244".force_encoding('UTF-8'), :length => 10)
   end
+  
+  def test_truncate_does_not_modify_the_options_hash
+    options = { :length => 10 }
+    passed_options = options.dup
+    truncate("some text", passed_options)
+    assert_equal options, passed_options
+  end
 
   def test_highlight_should_be_html_safe
     assert highlight("This is a beautiful morning", "beautiful").html_safe?
@@ -182,6 +203,13 @@ class TextHelperTest < ActionView::TestCase
       highlight("<div>abc div</div>", "div", :highlighter => '<b>\1</b>')
     )
   end
+  
+  def test_highlight_does_not_modify_the_options_hash
+    options = { :highlighter => '<b>\1</b>', :sanitize => false }
+    passed_options = options.dup
+    highlight("<div>abc div</div>", "div", passed_options)
+    assert_equal options, passed_options
+  end
 
   def test_excerpt
     assert_equal("...is a beautiful morn...", excerpt("This is a beautiful morning", "beautiful", :radius => 5))
@@ -228,6 +256,13 @@ class TextHelperTest < ActionView::TestCase
   def test_excerpt_with_utf8
     assert_equal("...\357\254\203ciency could not be...".force_encoding('UTF-8'), excerpt("That's why e\357\254\203ciency could not be helped".force_encoding('UTF-8'), 'could', :radius => 8))
   end
+  
+  def test_excerpt_does_not_modify_the_options_hash
+    options = { :omission => "[...]",:radius => 5 }
+    passed_options = options.dup
+    excerpt("This is a beautiful morning", "beautiful", passed_options)
+    assert_equal options, passed_options
+  end
 
   def test_word_wrap
     assert_equal("my very very\nvery long\nstring", word_wrap("my very very very long string", :line_width => 15))
@@ -236,6 +271,13 @@ class TextHelperTest < ActionView::TestCase
   def test_word_wrap_with_extra_newlines
     assert_equal("my very very\nvery long\nstring\n\nwith another\nline", word_wrap("my very very very long string\n\nwith another line", :line_width => 15))
   end
+  
+  def test_word_wrap_does_not_modify_the_options_hash
+    options = { :line_width => 15 }
+    passed_options = options.dup
+    word_wrap("some text", passed_options)
+    assert_equal options, passed_options
+  end
 
   def test_pluralization
     assert_equal("1 count", pluralize(1, "count"))