Add testing for examples in README, fix things.

Also led to extending a load more tests. - Rxhp::AttributeValidator::ValidationError is now Rxhp::ScriptError - HtmlSingletonElement raises a Rxhp::ValidationError if there are children - Symbols with underscores work correctly for boolean attribute names and values - consecutive strings children are concatenated - ditto if they're wrapped inside Rxhp::Fragment - validate in blocks too, not just at render and creation
fredemmott · Jan 22, 2012 · c5e1b8e · c5e1b8e
1 parent 89dbea0
commit c5e1b8e
Show file tree

Hide file tree

Showing 16 changed files with 423 additions and 55 deletions.
diff --git a/README.md b/README.md
@@ -90,7 +90,7 @@ If Rxhp::Html comes across a string, it escapes it - no exceptions.
 * There is no raw text function - you'd need to subclass Rxhp::Element and
   reimplement render()
 
-Attribute validation
+Attribute Validation
 --------------------
 
 This is fine:
@@ -126,8 +126,8 @@ pairs though - for example, this it won't spot the problem here:
 </ul>
 ```
 
-Validates singleton elements
-----------------------------
+Singleton Validation
+--------------------
 
 You'll get exceptions for things like this:
 
@@ -165,13 +165,15 @@ end
 Just by changing the render flags, you can get XHTML...
 
 ```html
-<!DOCTYPE HTML PUBLIC
+<!DOCTYPE html PUBLIC
   "-//W3C//DTD XHTML 1.0 Strict//EN"
   "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
 <html>
   <body>
     <div>
-      <p>foo</p>
+      <p>
+        foo
+      </p>
       <input type="checkbox" checked="checked" />
     </div>
   </body>
@@ -185,7 +187,9 @@ Just by changing the render flags, you can get XHTML...
 <html>
   <body>
     <div>
-      <p>foo</p>
+      <p>
+        foo
+      </p>
       <input type="checkbox" checked>
     </div>
   </body>
@@ -195,7 +199,7 @@ Just by changing the render flags, you can get XHTML...
 ... or still, technically, HTML:
 
 ```html
-<html><body><div><p>foo<br><input type="checkbox" checked></div>
+<html><body><div><p>foo<input type="checkbox" checked></div>
 ```
 
 How fast is it?
@@ -210,7 +214,7 @@ That's < 0.5ms per render in that example. Some other template systems are
 significantly faster, but this is still highly unlikely to be worrying in
 any modern webapp.
 
-Strings in blocks
+Strings in Blocks
 =================
 
 These examples raise an exception, as it's not possible to intercept the

diff --git a/lib/rxhp/attribute_validator.rb b/lib/rxhp/attribute_validator.rb
@@ -2,9 +2,6 @@
 
 module Rxhp
   module AttributeValidator
-    class ValidationError < Rxhp::ScriptError
-    end
-
     class UnacceptableAttributeError < ValidationError
       attr_reader :element, :attribute, :value
       def initialize element, attribute, value
@@ -63,17 +60,41 @@ def self.match? matcher, name, value
         matcher.any? { |x| match? x, name, value }
       when Hash
         matcher.any? do |name_matcher, value_matcher|
-          name_match = match?(name_matcher, name, nil)
-          value_match = match?(value_matcher, value, nil)
-          name_match && value_match
+          name_match = match_value?(name_matcher, name_from_symbol(name))
+          if name_match
+            match_value?(value_matcher, value)
+          else
+            false
+          end
         end
+      else
+        match_value? matcher, name
+      end
+    end
+
+    def self.match_value? matcher, value
+      if matcher == String
+        return (value.is_a? String) || (value.is_a? Symbol)
+      end
+
+      case matcher
+      when Array
+        matcher.any? {|x| match_value?(x, value) }
       when Symbol
-        matcher.to_s.gsub('_', '-') == name
+        name_from_symbol(matcher) == value
       else
-        matcher === name
+        if value.is_a? Symbol
+          (matcher === value.to_s) || (matcher === name_from_symbol(value))
+        else
+          matcher === value
+        end
       end
     end
 
+    def self.name_from_symbol symbol
+      symbol.to_s.gsub('_', '-')
+    end
+
     def self.included(klass)
       klass.extend(ClassMethods)
       class << klass

diff --git a/lib/rxhp/composable_element.rb b/lib/rxhp/composable_element.rb
@@ -12,8 +12,8 @@ module Rxhp
   class ComposableElement < Rxhp::Element
     include Rxhp::AttributeValidator
 
-    def initialize *args
-      super *args
+    def validate!
+      super
       validate_attributes!
     end
 
@@ -23,7 +23,7 @@ def initialize *args
     # This calls compose, provides the 'yield' magic, and callls render on
     # the output.
     def render options = {}
-      validate_attributes!
+      validate!
       self.compose do
         # Allow 'yield' to embed all children
         self.children.each do |child|

diff --git a/lib/rxhp/element.rb b/lib/rxhp/element.rb
@@ -14,12 +14,26 @@ class Element
     def initialize attributes = {}
       @attributes = attributes
       @children = Array.new
+      validate!
     end
 
     def children?
       !children.empty?
     end
 
+    def valid?
+      begin
+        validate!
+        true
+      rescue Rxhp::ValidationError
+        false
+      end
+    end
+
+    def validate!
+      # no-op
+    end
+
     # Return a flat HTML string for this element and all its' decendants.
     #
     # You probably don't want to implement this yourself - interesting
@@ -41,9 +55,23 @@ def render_string string, options
     def render_children options = {}
       return if children.empty?
 
-      children.map{ |child| render_child(child, options) }.join
+      flattened_children.map{ |child| render_child(child, options) }.join
+    end
+
+    # Fill default options
+    def fill_options options
+      {
+        :pretty => true,
+        :format => Rxhp::HTML_FORMAT,
+        :skip_doctype => false,
+        :doctype => Rxhp::HTML_5,
+        :depth => 0,
+        :indent => 2,
+      }.merge(options)
     end
 
+    private
+
     def render_child child, options
       case child
       when Element
@@ -57,16 +85,28 @@ def render_child child, options
       end
     end
 
-    # Fill default options
-    def fill_options options
-      {
-        :pretty => true,
-        :format => Rxhp::HTML_FORMAT,
-        :skip_doctype => false,
-        :doctype => Rxhp::HTML_5,
-        :depth => 0,
-        :indent => 2,
-      }.merge(options)
+    def flattened_children
+      no_frags = []
+      children.each do |node|
+        if node.is_a? Rxhp::Fragment
+          no_frags += node.children
+        else
+          no_frags.push node
+        end
+      end
+
+      previous = nil
+      no_consecutive_strings = []
+      no_frags.each do |node|
+        if node.is_a?(String) && previous.is_a?(String)
+          previous << node
+        else
+          no_consecutive_strings.push node
+          previous = node
+        end
+      end
+
+      no_consecutive_strings
     end
   end
 end
diff --git a/lib/rxhp/error.rb b/lib/rxhp/error.rb
@@ -4,4 +4,7 @@ class Error < ::StandardError
 
   class ScriptError < ::ScriptError
   end
+
+  class ValidationError < Rxhp::ScriptError
+  end
 end
diff --git a/lib/rxhp/html_element.rb b/lib/rxhp/html_element.rb
@@ -30,21 +30,21 @@ class HtmlElement < Element
     accept_attributes Rxhp::Html::GLOBAL_ATTRIBUTES
     accept_attributes Rxhp::Html::GLOBAL_EVENT_HANDLERS
 
-    def initialize *args
-      super *args
-      validate_attributes!
-    end
-
     def tag_name
       raise NotImplementedError.new
     end
 
+    def validate!
+      super
+      validate_attributes!
+    end
+
     # Render the element.
     #
     # Pays attention to the formatter type, doctype, pretty print options,
     # etc.
     def render options = {}
-      validate_attributes!
+      validate!
       options = fill_options(options)
 
       open = render_open_tag(options)
@@ -98,17 +98,20 @@ def render_open_tag options
       out = '<' + tag_name
       unless attributes.empty?
         attributes.each do |name,value|
+          name = name.to_s.gsub('_', '-') if name.is_a? Symbol
+          value = value.to_s.gsub('_', '-') if value.is_a? Symbol
+
           case value
           when false
             next
           when true
             if options[:format] == Rxhp::XHTML_FORMAT
-              out += ' ' + name.to_s + '="' + name.to_s + '"'
+              out += ' ' + name + '="' + name + '"'
             else
               out += ' ' + name
             end
           else
-            out += ' ' + name.to_s + '="' + html_escape(value.to_s) + '"'
+            out += ' ' + name.to_s + '="' + html_escape(value) + '"'
           end
         end
       end

diff --git a/lib/rxhp/html_singleton_element.rb b/lib/rxhp/html_singleton_element.rb
@@ -11,11 +11,12 @@ module Rxhp
   # - people don't expect to see them
   # - in XHTML, the opening tag will have been self closing.
   class HtmlSingletonElement < HtmlElement
-    def render *args
-      unless children.empty?
-        raise Rxhp::ScriptError.new('Singleton element has children')
-      end
-      super *args
+    class HasChildrenError < Rxhp::ValidationError
+    end
+
+    def validate!
+      super
+      raise HasChildrenError.new unless children.empty?
     end
 
     protected

diff --git a/lib/rxhp/scope.rb b/lib/rxhp/scope.rb
@@ -78,8 +78,9 @@ def self.define_element name, klass, namespace
                 "In a block, use the 'text' method to include Strings"
               )
             end
-            nil
           end
+          element.validate! if element.respond_to?(:validate!)
+          nil
         end
         element
       end

diff --git a/spec/attribute_validator_spec.rb b/spec/attribute_validator_spec.rb
@@ -18,17 +18,39 @@ def match? *args
       it 'should not allow a suffix match against the name' do
         match?('foo', 'barfoo', 'baz').should be_false
       end
+
+      it 'should allow a symbol match against the name' do
+        match?('foo', :foo, 'bar').should be_true
+      end
+    end
+
+    context 'with a matcher of String' do
+      it 'should match an arbitrary string' do
+        match?(String, 'foo', nil).should be_true
+      end
+
+      it 'should not match a number' do
+        match?(String, 1, nil).should be_false
+      end
+
+      it 'should match an arbitrary symbol' do
+        match?(String, :foo, nil).should be_true
+      end
     end
 
     context 'with a symbol matcher' do
       it 'should allow an exact match against the name' do
         match?(:foo, 'foo', 'bar').should be_true
       end
 
-      it 'should treat underscores as hyphens' do
+      it 'should treat underscores as hyphens in Symbol matchers' do
         match?(:foo_bar, 'foo-bar', 'baz').should be_true
       end
 
+      it 'should treat underscores as hyphens in Symbol names' do
+        match?('foo-bar', :foo_bar, 'baz').should be_true
+      end
+
       it 'should not allow a prefix match against the name' do
         match?(:foo, 'foobar', 'baz').should be_false
       end
@@ -92,6 +114,14 @@ def match? *args
         match?({'foo' => 'bar'}, 'foo', 'baz').should be_false
       end
 
+      it 'should accept accept a symbol for a fixed string value' do
+        match?({'foo' => 'bar'}, 'foo', :bar).should be_true
+      end
+
+      it 'should accept accept a symbol for an arbitrary String value' do
+        match?({'foo' => String}, 'foo', :bar).should be_true
+      end
+
       it 'should accept symbols for name matchers' do
         match?({:foo => 'bar'}, 'foo', 'bar').should be_true
         match?({:baz => 'bar'}, 'foo', 'bar').should be_false
@@ -168,7 +198,7 @@ def match? *args
 
     describe Rxhp::AttributeValidator do
       it 'should be a descendent of Rxhp::ScriptError' do
-        ancestors = Rxhp::AttributeValidator::ValidationError.ancestors
+        ancestors = Rxhp::ValidationError.ancestors
         ancestors.should include Rxhp::ScriptError
         ancestors.should include ::ScriptError
       end
@@ -179,7 +209,7 @@ def match? *args
         @instance.attributes['foo'] = 'bar'
         lambda do
           @instance.validate_attributes!
-        end.should raise_error(Rxhp::AttributeValidator::ValidationError)
+        end.should raise_error(Rxhp::ValidationError)
       end
     end