Do not convert newlines to HTML br element for 'block tags' (#31)

lib/ruby-bbcode/tag_collection.rb

@@ -17,7 +17,7 @@ def to_bbcode(tags)
     # This method is vulnerable to stack-level-too-deep scenarios where >=1,200 tags are being parsed.
     # But that scenario can be mitigated by splitting up the tags.  bbtree = { :nodes => [900tags, 1000tags] }, the work
     # for that bbtree can be split up into two passes, do the each node one at a time.  I'm not coding that though, it's pointless, just a thought though
-    def to_code(tags, template)
+    def to_code(tags, template, parent_node = nil)
       output_string = ''
       each do |node|
         if node.type == :tag
@@ -33,13 +33,13 @@ def to_code(tags, template)
           output_string << t.opening_part
 
           # invoke "recursive" call if this node contains child nodes
-          output_string << node.children.to_code(tags, template) if node.has_children? # FIXME:  Don't use recursion, it can lead to stack-level-too-deep errors for large volumes?
+          output_string << node.children.to_code(tags, template, node) if node.has_children? # FIXME:  Don't use recursion, it can lead to stack-level-too-deep errors for large volumes?
 
           t.inlay_closing_part!
 
           output_string << t.closing_part
         elsif node.type == :text
-          output_string << template.convert_text(node)
+          output_string << template.convert_text(node, parent_node)
         end
       end
 

lib/ruby-bbcode/tag_info.rb

@@ -3,6 +3,12 @@ module RubyBBCode
   # This class was made mostly just to keep track of all of the confusing the logic conditions that are checked.
   #
   class TagInfo
+    COMPLETE_MATCH = 0
+    CLOSING_MATCH = 2
+    TAG_MATCH = 3
+    TAG_PARAM_MATCH = 5
+    WHITESPACE_AFTER_TAG = 9
+
     def initialize(tag_info, dictionary)
       @tag_data = find_tag_info(tag_info, dictionary)
     end
@@ -23,6 +29,11 @@ def text
       @tag_data[:text]
     end
 
+    # Returns the whitespace that was available directly after the tag definition
+    def whitespace
+      @tag_data[:whitespace]
+    end
+
     # Returns the type of the cuvvrent tag/node, which is either :opening_tag, :closing_tag, or :text
     def type
       return :opening_tag if element_is_opening_tag?
@@ -88,7 +99,8 @@ def invalid_quick_param?
     def default_tag_info(tag_info)
       {
         errors: [],
-        complete_match: tag_info[0]
+        complete_match: tag_info[COMPLETE_MATCH],
+        whitespace: tag_info[WHITESPACE_AFTER_TAG]
       }
     end
 
@@ -97,10 +109,10 @@ def default_tag_info(tag_info)
     # Returns the tag hash
     def find_tag_info(tag_info, dictionary)
       ti = default_tag_info(tag_info)
-      ti[:is_tag] = (tag_info[0].start_with? '[')
+      ti[:is_tag] = (tag_info[COMPLETE_MATCH].start_with? '[')
       if ti[:is_tag]
-        ti[:closing_tag] = (tag_info[2] == '/')
-        ti[:tag] = tag_info[3].to_sym.downcase
+        ti[:closing_tag] = (tag_info[CLOSING_MATCH] == '/')
+        ti[:tag] = tag_info[TAG_MATCH].to_sym.downcase
         ti[:params] = {}
         @definition = dictionary[ti[:tag]]
         if !tag_in_dictionary?
@@ -110,12 +122,12 @@ def find_tag_info(tag_info, dictionary)
           ti = default_tag_info(tag_info)
           ti[:is_tag] = false
           ti[:text] = if RubyBBCode.configuration.ignore_unknown_tags == :text
-                        tag_info[0]
+                        tag_info[COMPLETE_MATCH]
                       else
                         ''
                       end
-        elsif (tag_info[5][0] == '=') && can_have_quick_param?
-          quick_param = tag_info[5][1..-1]
+        elsif (tag_info[TAG_PARAM_MATCH][0] == '=') && can_have_quick_param?
+          quick_param = tag_info[TAG_PARAM_MATCH][1..-1]
           # Get list of parameter values and add them as (regular) parameters
           value_array = quick_param.scan(@definition[:quick_param_format])[0]
           if value_array.nil?
@@ -126,17 +138,17 @@ def find_tag_info(tag_info, dictionary)
               ti[:params][param_tokens[i][:token]] = value
             end
           end
-        elsif tag_info[5][0] == "\s"
+        elsif tag_info[TAG_PARAM_MATCH][0] == "\s"
           regex_string = '((\w+)=([\w#]+)) | ((\w+)="([^"]+)") | ((\w+)=\'([^\']+)\')'
-          tag_info[5].scan(/#{regex_string}/ix) do |param_info|
+          tag_info[TAG_PARAM_MATCH].scan(/#{regex_string}/ix) do |param_info|
             param = param_info[1] || param_info[4] || param_info[7]
             value = param_info[2] || param_info[5] || param_info[8]
             ti[:params][param.to_sym] = value
           end
         end
       else
         # Plain text
-        ti[:text] = tag_info[9]
+        ti[:text] = tag_info[COMPLETE_MATCH]
       end
       ti
     end

lib/ruby-bbcode/tag_sifter.rb

@@ -25,19 +25,22 @@ def valid?
     # once this tree is built, the to_html method can be invoked where the tree is finally
     # converted into HTML syntax.
     def process_text
-      regex_string = '((\[ (\/)? ( \* | (\w+)) ((=[^\[\]]+) | (\s\w+=\w+)* | ([^\]]*))? \]) | ([^\[]+))'
+      regex_string = '((\[ (\/)? ( \* | (\w+)) ((=[^\[\]]+) | (\s\w+=\w+)* | ([^\]]*))? \] (\s*)) | ([^\[]+))'
       @text.scan(/#{regex_string}/ix) do |tag_info|
         @ti = TagInfo.new(tag_info, @dictionary)
 
         validate_element
 
         case @ti.type
         when :opening_tag
-          element = { is_tag: true, tag: @ti[:tag], definition: @ti.definition, errors: @ti[:errors], nodes: TagCollection.new }
+          element = { is_tag: true, tag: @ti[:tag], definition: @ti.definition, opening_whitespace: @ti[:whitespace], errors: @ti[:errors], nodes: TagCollection.new }
           element[:invalid_quick_param] = true if @ti.invalid_quick_param?
           element[:params] = get_formatted_element_params
 
-          @bbtree.retrogress_bbtree if self_closing_tag_reached_a_closer?
+          if self_closing_tag_reached_a_closer?
+            transfer_whitespace_to_closing_tag
+            @bbtree.retrogress_bbtree
+          end
 
           @bbtree.build_up_new_tag(element)
 
@@ -73,12 +76,16 @@ def process_text
 
           create_text_element
         when :closing_tag
+          @bbtree.current_node[:closing_whitespace] = @ti[:whitespace]
           if @ti[:wrong_closing]
             # Convert into text, so it
             @ti.handle_tag_as_text
             create_text_element
           else
-            @bbtree.retrogress_bbtree if parent_of_self_closing_tag? && within_open_tag?
+            if parent_of_self_closing_tag? && within_open_tag?
+              transfer_whitespace_to_closing_tag
+              @bbtree.retrogress_bbtree
+            end
             @bbtree.retrogress_bbtree
           end
         end
@@ -90,6 +97,25 @@ def process_text
 
     private
 
+    def transfer_whitespace_to_closing_tag
+      last_text_node = node_last_text(parent_tag)
+      unless last_text_node.nil?
+        last_text_node[:text].scan(/(\s+)$/) do |result|
+          parent_tag[:closing_whitespace] = result[0]
+          last_text_node[:text] = last_text_node[:text][0..-result[0].length - 1]
+        end
+      end
+    end
+
+    # Return the node that holds the last piece of text for the given node (self or child)
+    def node_last_text(node)
+      return node if node.type == :text
+      return node_last_text(node.children[-1]) unless node.children.empty?
+
+      # node does not hold text
+      nil
+    end
+
     def set_multi_tag_to_actual_tag
       # Try to find the actual tag
       tag = get_actual_tag
@@ -254,6 +280,7 @@ def last_tag_fit_in_this_tag?
       end
       false
     end
+
     # This validation is for text elements with between text
     # that might be construed as a param.
     # The validation code checks if the params match constraints

lib/ruby-bbcode/templates/bbcode_errors_template.rb

@@ -14,12 +14,12 @@ class BBCodeErrorsTemplate
     def initialize(node)
       @node = node
       @tag_definition = node.definition # tag_definition
-      @opening_part = "[#{node[:tag]}#{node.allow_params? ? '%param%' : ''}]"
+      @opening_part = "[#{node[:tag]}#{node.allow_params? ? '%param%' : ''}]" + add_whitespace(node[:opening_whitespace])
       @opening_part = "<span class='bbcode_error' #{BBCodeErrorsTemplate.error_attribute(@node[:errors])}>#{@opening_part}</span>" unless @node[:errors].empty?
-      @closing_part = "[/#{node[:tag]}]"
+      @closing_part = "[/#{node[:tag]}]" + add_whitespace(node[:closing_whitespace])
     end
 
-    def self.convert_text(node)
+    def self.convert_text(node, _parent_node)
       # Keep the text as it was
       return "<span class='bbcode_error' #{error_attribute(node[:errors])}>#{node[:text]}</span>" unless node[:errors].empty?
 
@@ -52,6 +52,10 @@ def closing_part
 
     private
 
+    def add_whitespace(whitespace)
+      whitespace || ''
+    end
+
     def get_between
       return @node[:between] if @tag_definition[:require_between] && @node[:between]
 

lib/ruby-bbcode/templates/html_template.rb

@@ -10,17 +10,26 @@ class HtmlTemplate
 
     def initialize(node)
       @node = node
-      @tag_definition = node.definition # tag_definition
-      @opening_part = node.definition[:html_open].dup
-      @closing_part = node.definition[:html_close].dup
+      @tag_definition = node.definition
+      @opening_part = node.definition[:html_open] + add_whitespace(:opening_whitespace)
+      @closing_part = node.definition[:html_close] + add_whitespace(:closing_whitespace)
     end
 
     # Newlines are converted to html <br /> syntax before being returned.
-    def self.convert_text(node)
+    def self.convert_text(node, parent_node)
       return '' if node[:text].nil?
 
-      # convert_newlines_to_br
-      node[:text].gsub("\r\n", "\n").gsub("\n", "<br />\n")
+      text = node[:text]
+      whitespace = ''
+
+      if !parent_node.nil? && parent_node.definition[:block_tag]
+        # Strip EOL whitespace, so it does not get converted
+        text.scan(/(\s+)$/) do |result|
+          whitespace = result[0]
+          text = text[0..-result[0].length - 1]
+        end
+      end
+      convert_newlines(text) + whitespace
     end
 
     def inlay_between_text!
@@ -46,8 +55,20 @@ def remove_unused_tokens!
       end
     end
 
+    def self.convert_newlines(text)
+      text.gsub("\r\n", "\n").gsub("\n", "<br />\n")
+    end
+
     private
 
+    def add_whitespace(key)
+      whitespace = @node[key]
+      return '' if whitespace.nil?
+
+      whitespace = HtmlTemplate.convert_newlines(whitespace) unless @tag_definition[:block_tag]
+      whitespace
+    end
+
     # Return true if the between text is needed as param
     def between_text_as_param?
       @tag_definition[:require_between]

lib/tags/tags.rb

@@ -33,37 +33,42 @@ module Tags
       :ul => {
         html_open: '<ul>', html_close: '</ul>',
         description: 'Unordered list',
+        block_tag: true,
         example: '[ul][li]List item[/li][li]Another list item[/li][/ul].',
         only_allow: [:li, '*'.to_sym]
       },
       :code => {
         html_open: '<pre>', html_close: '</pre>',
         description: 'Code block with mono-spaced text',
-        example: 'This is [code]mono-spaced code[/code].'
+        example: 'This is [code]mono-spaced code[/code].',
+        block_tag: true
       },
       :ol => {
         html_open: '<ol>', html_close: '</ol>',
         description: 'Ordered list',
         example: '[ol][li]List item[/li][li]Another list item[/li][/ol].',
+        block_tag: true,
         only_allow: [:li, '*'.to_sym]
       },
       :li => {
         html_open: '<li>', html_close: '</li>',
         description: 'List item',
         example: '[ul][li]List item[/li][li]Another list item[/li][/ul].',
+        block_tag: true,
         only_in: %i[ul ol]
       },
       :list => {
         html_open: '<ul>', html_close: '</ul>',
         description: 'Unordered list',
         example: '[list][*]List item[*]Another list item[/list].',
+        block_tag: true,
         only_allow: ['*'.to_sym]
       },
       '*'.to_sym => {
         html_open: '<li>', html_close: '</li>',
         description: 'List item',
         example: '[list][*]List item[*]Another list item[/list].',
-        self_closable: true,
+        self_closable: true, block_tag: true,
         only_in: %i[list ul ol]
       },
       :img => {
@@ -72,7 +77,7 @@ module Tags
         example: '[img]http://www.google.com/intl/en_ALL/images/logo.gif[/img].',
         only_allow: [],
         require_between: true,
-        allow_quick_param: true, allow_between_as_param: false,
+        allow_quick_param: true, allow_between_as_param: false, block_tag: true,
         quick_param_format: /^(\d+)x(\d+)$/,
         param_tokens: [{ token: :width, prefix: 'width="', postfix: '" ', optional: true },
                        { token: :height, prefix: 'height="', postfix: '" ', optional: true }],
@@ -92,7 +97,7 @@ module Tags
         html_open: '<div class="quote">%author%', html_close: '</div>',
         description: 'Quote another person',
         example: '[quote]BBCode is great[/quote]',
-        allow_quick_param: true, allow_between_as_param: false,
+        allow_quick_param: true, allow_between_as_param: false, block_tag: true,
         quick_param_format: /(.*)/,
         param_tokens: [{ token: :author, prefix: '<strong>', postfix: ' wrote:</strong>', optional: true }]
       },
@@ -119,7 +124,7 @@ module Tags
         example: '[youtube]E4Fbk52Mk1w[/youtube]',
         only_allow: [],
         url_matches: [/youtube\.com.*[v]=([^&]*)/, %r{youtu\.be/([^&]*)}, %r{y2u\.be/([^&]*)}],
-        require_between: true,
+        require_between: true, block_tag: true,
         param_tokens: [
           { token: :width, optional: true, default: 400 },
           { token: :height, optional: true, default: 320 }
@@ -132,7 +137,7 @@ module Tags
         example: '[vimeo]http://vimeo.com/46141955[/vimeo]',
         only_allow: [],
         url_matches: [%r{vimeo\.com/([^&]*)}],
-        require_between: true,
+        require_between: true, block_tag: true,
         param_tokens: [
           { token: :width, optional: true, default: 400 },
           { token: :height, optional: true, default: 320 }

test/ruby_bbcode_bbcode_test.rb

@@ -50,14 +50,17 @@ def test_center
 
   def test_ordered_list
     assert_equal '[ol][li]item 1[/li][li]item 2[/li][/ol]', '[ol][li]item 1[/li][li]item 2[/li][/ol]'.bbcode_show_errors
+    assert_equal "[ol]\n[li]item 1[/li]\n[li]item 2[/li]\n[/ol]", "[ol]\n[li]item 1[/li]\n[li]item 2[/li]\n[/ol]".bbcode_show_errors
   end
 
   def test_unordered_list
     assert_equal '[ul][li]item 1[/li][li]item 2[/li][/ul]', '[ul][li]item 1[/li][li]item 2[/li][/ul]'.bbcode_show_errors
+    assert_equal "[ul]\n\t[li]item 1[/li]\n\t[li]item 2[/li]\n[/ul]", "[ul]\n\t[li]item 1[/li]\n\t[li]item 2[/li]\n[/ul]".bbcode_show_errors
   end
 
   def test_list_common_syntax
     assert_equal '[list][*]item 1[/*][*]item 2[/*][/list]', '[list][*]item 1[*]item 2[/list]'.bbcode_show_errors
+    assert_equal "[list]\n[*]item 1[/*]\n[*]item 2[/*]\n[/list]", "[list]\n[*]item 1\n[*]item 2\n[/list]".bbcode_show_errors
   end
 
   def test_list_common_syntax_explicit_closing
@@ -78,6 +81,7 @@ def test_whitespace_in_only_allowed_tags
 
   def test_quote
     assert_equal '[quote]quoting[/quote]', '[quote]quoting[/quote]'.bbcode_show_errors
+    assert_equal "[quote]\nquoting\n[/quote]", "[quote]\nquoting\n[/quote]".bbcode_show_errors
     assert_equal '[quote author=someone]quoting[/quote]', '[quote=someone]quoting[/quote]'.bbcode_show_errors
     assert_equal '[quote author=Kitten][quote author=creatiu]f1[/quote]f2[/quote]',
                  '[quote author=Kitten][quote=creatiu]f1[/quote]f2[/quote]'.bbcode_show_errors