Clean up multiple calls to #stringify_keys in TagHelper, add better documentation and testing for TagHelper. Closes #6394 [Bob Silva]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@5857 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

actionpack/CHANGELOG

@@ -1,5 +1,7 @@
 *SVN*
 
+* Clean up multiple calls to #stringify_keys in TagHelper, add better documentation and testing for TagHelper.  Closes #6394 [Bob Silva]
+
 * [DOCS] fix reference to ActionController::Macros::AutoComplete for #text_field_with_auto_complete. Closes #2578 [Jan Prill]
 
 * Make sure html_document is reset between integration test requests. [ctm]

actionpack/lib/action_view/helpers/tag_helper.rb

@@ -2,30 +2,48 @@ require 'cgi'
 require 'erb'
 
 module ActionView
-  module Helpers
-    # This is poor man's Builder for the rare cases where you need to programmatically make tags but can't use Builder.
+  module Helpers #:nodoc:
+    # Use these methods to generate HTML tags programmatically when you can't use
+    # a Builder. By default, they output XHTML compliant tags.
     module TagHelper
       include ERB::Util
 
-      # Examples:
-      # * <tt>tag("br") => <br /></tt>
-      # * <tt>tag("input", { "type" => "text"}) => <input type="text" /></tt>
+      # Returns an empty HTML tag of type +name+ which by default is XHTML 
+      # compliant. Setting +open+ to true will create an open tag compatible 
+      # with HTML 4.0 and below. Add HTML attributes by passing an attributes 
+      # hash to +options+. For attributes with no value like (disabled and 
+      # readonly), give it a value of true in the +options+ hash. You can use
+      # symbols or strings for the attribute names.
+      #
+      #   tag("br")
+      #    # => <br />
+      #   tag("br", nil, true)
+      #    # => <br>
+      #   tag("input", { :type => 'text', :disabled => true }) 
+      #    # => <input type="text" disabled="disabled" />
       def tag(name, options = nil, open = false)
-        "<#{name}#{tag_options(options.stringify_keys) if options}" + (open ? ">" : " />")
+        "<#{name}#{tag_options(options) if options}" + (open ? ">" : " />")
       end
 
-      # Examples:
-      # * <tt>content_tag(:p, "Hello world!") => <p>Hello world!</p></tt>
-      # * <tt>content_tag(:div, content_tag(:p, "Hello world!"), :class => "strong") => </tt>
-      #   <tt><div class="strong"><p>Hello world!</p></div></tt>
+      # Returns an HTML block tag of type +name+ surrounding the +content+. Add
+      # HTML attributes by passing an attributes hash to +options+. For attributes 
+      # with no value like (disabled and readonly), give it a value of true in 
+      # the +options+ hash. You can use symbols or strings for the attribute names.
       #
-      # ERb example:
-      #  <% content_tag :div, :class => "strong" do -%>
-      #    Hello world!
-      #  <% end -%>
+      #   content_tag(:p, "Hello world!")
+      #    # => <p>Hello world!</p>
+      #   content_tag(:div, content_tag(:p, "Hello world!"), :class => "strong")
+      #    # => <div class="strong"><p>Hello world!</p></div>
+      #   content_tag("select", options, :multiple => true)
+      #    # => <select multiple="multiple">...options...</select>
       #
-      # Will output:
-      #   <div class="strong"><p>Hello world!</p></div>
+      # Instead of passing the content as an argument, you can also use a block
+      # in which case, you pass your +options+ as the second parameter.
+      #
+      #   <% content_tag :div, :class => "strong" do -%>
+      #     Hello world!
+      #   <% end -%>
+      #    # => <div class="strong"><p>Hello world!</p></div>
       def content_tag(name, content_or_options_with_block = nil, options = nil, &block)
         if block_given?
           options = content_or_options_with_block if content_or_options_with_block.is_a?(Hash)
@@ -37,27 +55,28 @@ module ActionView
         end
       end
 
-      # Returns a CDATA section for the given +content+.  CDATA sections
+      # Returns a CDATA section with the given +content+.  CDATA sections
       # are used to escape blocks of text containing characters which would
       # otherwise be recognized as markup. CDATA sections begin with the string
-      # <tt>&lt;![CDATA[</tt> and end with (and may not contain) the string 
-      # <tt>]]></tt>. 
+      # <tt><![CDATA[</tt> and end with (and may not contain) the string <tt>]]></tt>.
+      #
+      #   cdata_section("<hello world>")
+      #    # => <![CDATA[<hello world>]]>
       def cdata_section(content)
         "<![CDATA[#{content}]]>"
       end
 
-      # Escapes a given string, while leaving any currently escaped entities alone.
+      # Returns the escaped +html+ without affecting existing escaped entities.
       #
       #   escape_once("1 > 2 &amp; 3")
-      #   # => "1 &lt; 2 &amp; 3"
-      #
+      #    # => "1 &lt; 2 &amp; 3"
       def escape_once(html)
         fix_double_escape(html_escape(html.to_s))
       end
 
       private
         def content_tag_string(name, content, options)
-          tag_options = options ? tag_options(options.stringify_keys) : ""
+          tag_options = options ? tag_options(options) : ""
           "<#{name}#{tag_options}>#{content}</#{name}>"
         end
       

actionpack/test/template/tag_helper_test.rb

@@ -7,8 +7,9 @@ class TagHelperTest < Test::Unit::TestCase
   include ActionView::Helpers::CaptureHelper
 
   def test_tag
-    assert_equal "<p class=\"show\" />", tag("p", "class" => "show")
-    assert_equal tag("p", "class" => "show"), tag("p", :class => "show")
+    assert_equal "<br />", tag("br")
+    assert_equal "<br clear=\"left\" />", tag(:br, :clear => "left")
+    assert_equal "<br>", tag("br", nil, true)
   end
 
   def test_tag_options