Skip to content
Browse files

Fixed a fairly interesting unwinding bug. The logic that was being us…

…ed to detect list of elements was to simplistict. I am still not sure it is 100% correct, though it is much better
  • Loading branch information...
1 parent 6643ebb commit a767f970408288b71d59b7c74fd68105e29ea93d @douglasjsellers committed Apr 15, 2012
View
61 lib/nodes/herb_node_retaining_text_node.rb
@@ -59,7 +59,7 @@ def extract_text( text_extractor, node_tree )
def can_be_exploded?
# it is possible that this node contains only non text nodes. If
# there is no text within this combined node then this should be uncombined
- find_all_non_matched_tags_and_white_space(nodes).empty? || contains_only_non_text_and_non_method_nodes?
+ contains_list_of_elements? || contains_only_non_text_and_non_method_nodes?
end
def can_be_combined?
@@ -89,10 +89,45 @@ def contains_only_non_text_and_non_method_nodes?
found_text = false
nodes.each do |current_node|
found_text = true if current_node.is_a?( TextNode ) && !current_node.is_a?(MethodCallNode)
-
end
!found_text
end
+
+ def contains_list_of_elements?
+ # Two cases, first there could be an outer element that wraps the
+ # list, if this is true we want to strip it.
+ # After the outer elements are stripped then we want to walk
+ # through and find all of the top level elements - where a top
+ # level element is defined as anything that isn't nested further.
+
+ nested_level = 0
+ nested_nodes = [ [] ]
+ nodes.each do |current_node|
+ if( current_node.node_name == "html_start_tag" )
+ nested_nodes[nested_level] << current_node
+ nested_level += 1
+ nested_nodes[ nested_level ] = [] if nested_nodes[nested_level].nil?
+ elsif( current_node.node_name == "html_end_tag" )
+ nested_level -= 1
+ nested_nodes[ nested_level ] << current_node
+ elsif( current_node.is_a?( TextNode ) && current_node.contains_alpha_characters? )
+ nested_nodes[ nested_level ] << current_node
+ end
+ end
+
+ nested_level = 0
+ to_return = true
+ while( nested_nodes[nested_level].size == 2 && nested_nodes[nested_level].first.node_name == "html_start_tag" && nested_nodes[nested_level].last.node_name == "html_end_tag" )
+ nested_level += 1
+ end
+
+ nested_nodes[nested_level].each do |node_at_node_level|
+ to_return = false if node_at_node_level.is_a?(TextNode)
+ end
+
+ return to_return
+ end
+
def extract_leading_tag
node = find_first_non_whitespace_node
@@ -117,26 +152,4 @@ def extract_trailing_tag
end
- def find_all_non_matched_tags_and_white_space( nodes )
- to_return = []
- search_node_name = nil
- nodes.each do |current_node|
- if( !current_node.respond_to?( :node_name ) )
- to_return << current_node
- elsif( search_node_name.nil? && current_node.node_name == "html_start_tag" )
- search_node_name = current_node.tag_name.text_value
- elsif( !search_node_name.nil? && current_node.node_name == "html_end_tag" )
- search_node_name = nil if search_node_name == current_node.tag_name.text_value
- elsif( search_node_name.nil? && current_node.is_a?(TextNode ) )
- if( current_node.contains_alpha_characters? )
- to_return << current_node
- end
- end
- end
-
- return to_return
- end
-
-
-
end
View
3 ...s/integration/result/erb/self_contained_span_followed_by_text_filled_span.erb.i18n.result
@@ -0,0 +1,3 @@
+ <span class="bd">
+ <%= t '.register_today' %>
+ </span>
View
3 ...s/integration/result/erb/self_contained_span_followed_by_text_filled_span.erb.tr8n.result
@@ -0,0 +1,3 @@
+ <span class="bd">
+ <%= tr( "[strong: Register today] to save your progress.", nil, { :strong => "<strong>{$0}</strong>" } ) %>
+ </span>
View
6 ...tegration/result/yml/self_contained_span_followed_by_text_filled_span.erb.yml.i18n.result
@@ -0,0 +1,6 @@
+en:
+ tests:
+ integration:
+ test:
+ self_contained_span_followed_by_text_filled_span:
+ register_today: "<strong>Register today</strong> to save your progress."
View
3 tests/integration/test/self_contained_span_followed_by_text_filled_span.erb
@@ -0,0 +1,3 @@
+ <span class="bd">
+ <strong>Register today</strong> to save your progress.
+ </span>
View
27 tests/specs/herb_node_retaining_text_node_spec.rb
@@ -157,8 +157,33 @@
node.add_all( erb_file.nodes )
node.can_be_exploded?.should == false
-
end
+
+ it "should be able to detect that something with pipe separators should be exploded" do
+ erb_file = ErbFile.from_string( "\n<a href='blah'>stuff</a>\n|\n<a href='two'>more</a>\n" )
+ node = HerbNodeRetainingTextNode.new
+ node.add_all( erb_file.nodes )
+
+ node.can_be_exploded?.should == true
+ end
+
+ it "should be able to detect that a list wrapped in a span should be exploded" do
+ erb_file = ErbFile.from_string( "<span>\n<a href='blah'>stuff</a>\n|\n<a href='two'>more</a>\n</span>" )
+ node = HerbNodeRetainingTextNode.new
+ node.add_all( erb_file.nodes )
+
+ node.can_be_exploded?.should == true
+ end
+
+ it "should be able to detect that a list containing a mixed html and text should not be exploded" do
+
+ erb_file = ErbFile.from_string( "<span>\n<strong>Register today</strong> to save your progress.\n</span>" )
+ node = HerbNodeRetainingTextNode.new
+ node.add_all( erb_file.nodes )
+
+ node.can_be_exploded?.should == false
+ end
+
end

0 comments on commit a767f97

Please sign in to comment.
Something went wrong with that request. Please try again.