]> git.wincent.com - wikitext.git/commitdiff
Fix obscure bug handling empty lines in PRE blocks
authorWincent Colaiuta <win@wincent.com>
Fri, 21 Mar 2008 00:52:20 +0000 (01:52 +0100)
committerWincent Colaiuta <win@wincent.com>
Fri, 21 Mar 2008 01:22:08 +0000 (02:22 +0100)
If a PRE block was marked up using an intial leading space then any
"empty" lines (ie. lines containing only a space) would incorrectly
close the block; if the block continued on subsequent lines then it
would be re-opened.

This commit adds a regression spec, fixes the bug, and tweaks the
integration spec because the fix exposed a dormant bug in the spec (an
extra space which should have altered the output).

The fix consists of a few parts: on seeing a CRLF we clear the line
buffer _before_ calling NEXT_TOKEN (otherwise whatever gets pushed onto
the line buffer by NEXT_TOKEN will get blown away and won't be taken
into consideration the next time around the loop); we drop the useless
NO_ITEM test (I don't believe this test can ever pass as by definition
we are guaranteed to have at least PRE or BLOCKQUOTE plus CRLF on the
line buffer at that point); and finally we special-case the situation
where the _only_ thing on the line is a PRE token (here we emit any
pending newline as required).

Some additional specs are included in pre_spec.rb to show the benefit
of this new handling.

Signed-off-by: Wincent Colaiuta <win@wincent.com>
ext/parser.c
spec/integration_spec.rb
spec/pre_spec.rb
spec/regressions_spec.rb

index 03b14890ddc862021c35485744211d7b23777caf..69d924f836d8fad0e4fb31ccc861a38c8b51a75d 100644 (file)
@@ -2159,6 +2159,7 @@ VALUE Wikitext_parser_parse(int argc, VALUE *argv, VALUE self)
                 break;
 
             case CRLF:
+                i = parser->pending_crlf;
                 parser->pending_crlf = Qfalse;
                 _Wikitext_rollback_failed_link(parser);             // if any
                 _Wikitext_rollback_failed_external_link(parser);    // if any
@@ -2170,27 +2171,33 @@ VALUE Wikitext_parser_parse(int argc, VALUE *argv, VALUE self)
                 }
                 else if (IN(PRE))
                 {
-                    // beware when nothing or BLOCKQUOTE on line buffer (not line stack!) prior to CRLF, that must be end of PRE block
-                    if (NO_ITEM(ary_entry(parser->line_buffer, -2)) || ary_entry(parser->line_buffer, -2) == BLOCKQUOTE)
+                    // beware when BLOCKQUOTE on line buffer (not line stack!) prior to CRLF, that must be end of PRE block
+                    if (ary_entry(parser->line_buffer, -2) == BLOCKQUOTE)
                         // don't emit in this case
                         _Wikitext_pop_from_stack_up_to(parser, parser->output, PRE, Qtrue);
                     else
                     {
+                        if (ary_entry(parser->line_buffer, -2) == PRE)
+                        {
+                             // only thing on line is the PRE: emit pending line ending (if we had one)
+                             if (i == Qtrue)
+                                 rb_str_cat(parser->output, parser->line_ending->ptr, parser->line_ending->len);
+                        }
+
+                        // clear these _before_ calling NEXT_TOKEN (NEXT_TOKEN adds to the line_buffer)
+                        ary_clear(parser->line);
+                        ary_clear(parser->line_buffer);
+
                         // peek ahead to see if this is definitely the end of the PRE block
                         NEXT_TOKEN();
                         type = token->type;
                         if (type != BLOCKQUOTE && type != PRE)
-                        {
                             // this is definitely the end of the block, so don't emit
                             _Wikitext_pop_from_stack_up_to(parser, parser->output, PRE, Qtrue);
-                        }
                         else
                             // potentially will emit
                             parser->pending_crlf = Qtrue;
 
-                        // delete the entire contents of the line scope stack and buffer
-                        ary_clear(parser->line);
-                        ary_clear(parser->line_buffer);
                         continue; // jump back to top of loop to handle token grabbed via lookahead
                     }
                 }
@@ -2251,6 +2258,12 @@ VALUE Wikitext_parser_parse(int argc, VALUE *argv, VALUE self)
                 break;
 
             case END_OF_FILE:
+                // special case for input like " foo\n " (see pre_spec.rb)
+                if (IN(PRE) &&
+                    ary_entry(parser->line_buffer, -2) == PRE &&
+                    parser->pending_crlf == Qtrue)
+                    rb_str_cat(parser->output, parser->line_ending->ptr, parser->line_ending->len);
+
                 // close any open scopes on hitting EOF
                 _Wikitext_rollback_failed_external_link(parser);    // if any
                 _Wikitext_rollback_failed_link(parser);             // if any
index 4f58a4ad322bd80543b6d0815c969bbbca0fdc8b..aa7080a32b638579a7aa04644a5f8d7577e6a06d 100755 (executable)
@@ -87,7 +87,7 @@ describe Wikitext::Parser, 'with large slab of input text' do
        notice how it can contain ''markup''
        which would '''otherwise''' have <tt>special</tt> meaning
        although explicit entities &copy; are passed through unchanged
-       
+      
       a normal paragraph again
       
       {{an_image.png}}
index e7a1aebf698cd9e5c464202d5d45dc1c8b9b1d0d..249fc9b0ead086cd898088c1b00726f2b66fa0cf 100755 (executable)
@@ -29,6 +29,33 @@ describe Wikitext::Parser, 'parsing PRE blocks' do
     @parser.parse(" foo\n bar").should == "<pre>foo\nbar</pre>\n"
   end
 
+  it 'should handle "empty" lines in the middle of multiline PRE blocks' do
+    input = dedent <<-END
+       foo
+       
+       bar
+    END
+    expected = dedent <<-END
+      <pre>foo
+      
+      bar</pre>
+    END
+    @parser.parse(input).should == expected
+  end
+
+  it 'should render an empty block for an empty PRE block' do
+    @parser.parse(' ').should == "<pre></pre>\n"
+  end
+
+  it 'should sanely handle a leading empty line' do
+    @parser.parse(" \n foo").should == "<pre>\nfoo</pre>\n"
+  end
+
+  it 'should sanely handle a trailing empty line' do
+    @parser.parse(" foo\n \n").should == "<pre>foo\n</pre>\n"
+    @parser.parse(" foo\n ").should == "<pre>foo\n</pre>\n"
+  end
+
   it 'should allow nesting inside a <blockquote> block' do
     # nesting inside single blockquotes
     @parser.parse(">  foo").should == "<blockquote>\n  <pre>foo</pre>\n</blockquote>\n"
index 0ea41dd6ce8d958649920e167d5101982dc55a31..7a3eeadbae5bb52da66e203a349cef47106d86d4 100755 (executable)
@@ -42,4 +42,23 @@ describe Wikitext::Parser, 'regressions' do
     END
     @parser.parse(input).should == expected
   end
+
+  # this one discovered in a real Rails application
+  it 'should allow empty lines in PRE blocks marked up with a leading space' do
+    input = dedent <<-END
+       # -d turns on debug mode: output is verbose, no actual changes are made to the log files
+       sudo logrotate -d /etc/logrotate.d/nginx
+       
+       # if the debug output looks good, proceed with a real rotation (-v turns on verbose output)
+       sudo logrotate -v /etc/logrotate.d/nginx
+    END
+    expected = dedent <<-END
+      <pre># -d turns on debug mode: output is verbose, no actual changes are made to the log files
+      sudo logrotate -d /etc/logrotate.d/nginx
+      
+      # if the debug output looks good, proceed with a real rotation (-v turns on verbose output)
+      sudo logrotate -v /etc/logrotate.d/nginx</pre>
+    END
+    @parser.parse(input).should == expected
+  end
 end