RedCloth master generates a spurios <li> element when a numbered or
undumbered list is followed by three or more newlines.
RedCloth.new("* one\n* two\n* three \n\n\n").to_html
#=>
"<ul>\n\t<li>one</li>\n\t<li>two</li>\n\t<li>three</li>\n</ul>\n<li>"
# note extra trailing <li> outside <ul>
A colleague generated a pull request with a new rspec test that exposes this
bug:
https://github.com/jgarber/redcloth/pull/1
I checked and the bug is present on master.
I just took a look at the ragel code but it''s been quite a while since
I''ve used ragel and it''s not quite as easy to parse as
Ruby ;-)
Seems like li_open in lib/redcloth/formatters/html.rb is being called
incorrectly when the thrid newline is parsed.
Here are the locations in the code where li_open is referenced:
$ ack --all-types li_open
ext/redcloth_scan/redcloth.h
202: ASET("type", "li_open");
ext/redcloth_scan/RedclothScanService.java
95: ASET("type", "li_open");
lib/redcloth/formatters/html.rb
50: def li_open(opts)
lib/redcloth/formatters/latex.rb
103: def li_open(opts)
ragel/redcloth_scan.java.rl
94: ASET("type", "li_open");
ragel/redcloth_scan.rb.rl
309: ASET("type", "li_open")
>RedCloth master generates a spurios <li> element when a numbered or undumbered list is followed by three or more newlines. > >RedCloth.new("* one\n* two\n* three \n\n\n").to_html >#=> "<ul>\n\t<li>one</li>\n\t<li>two</li>\n\t<li>three</li>\n</ul>\n<li>" ># note extra trailing <li> outside <ul> > >A colleague generated a pull request with a new rspec test that exposes this bug: > >https://github.com/jgarber/redcloth/pull/1 > >I checked and the bug is present on master. > >I just took a look at the ragel code but it''s been quite a while since I''ve used ragel and it''s not quite as easy to parse as Ruby ;-) > >Seems like li_open in lib/redcloth/formatters/html.rb is being called incorrectly when the thrid newline is parsed. > >Here are the locations in the code where li_open is referenced: > >$ ack --all-types li_open > > ext/redcloth_scan/redcloth.h > 202: ASET("type", "li_open"); > > ext/redcloth_scan/RedclothScanService.java > 95: ASET("type", "li_open"); > > lib/redcloth/formatters/html.rb > 50: def li_open(opts) > > lib/redcloth/formatters/latex.rb > 103: def li_open(opts) > > ragel/redcloth_scan.java.rl > 94: ASET("type", "li_open"); > > ragel/redcloth_scan.rb.rl > 309: ASET("type", "li_open") >Here''s the new spec test: it "should not add spurious li tags to the end of markup" do input = "* one\n* two\n* three \n\n" failing_input = "* one\n* two\n* three \n\n\n" RedCloth.new(input).to_html.should_not match /<li>$/ RedCloth.new(failing_input).to_html.should_not match /<li>$/ end When I run the spec test in the debugger: $ rspec -d spec/parser_spec.rb RedCloth::Formatters::HTML.li_open(opts) is called once for each of the three list items in the first test: RedCloth.new(input).to_html.should_not match /<li>$/ and then called again once for each list item in the second test: RedCloth.new(failing_input).to_html.should_not match /<li>$/ and then it is called a seventh time with this argument: {:type=>"li_open", :nest=>0, :text=>""} 50 def li_open(opts) 51 debugger => 52 "#{"\t" * opts[:nest]}<li#{pba(opts)}>#{opts[:text]}" 53 end values of argument opts: 1: {:type=>"li_open", :nest=>1, :text=>"one"} 2: {:type=>"li_open", :nest=>1, :text=>"two"} 3: {:type=>"li_open", :nest=>1, :text=>"three"} 4: {:type=>"li_open", :nest=>1, :text=>"one"} 5: {:type=>"li_open", :nest=>1, :text=>"two"} 6: {:type=>"li_open", :nest=>1, :text=>"three"} 7: {:type=>"li_open", :nest=>0, :text=>""} The nesting level is 0 and the text is an empty string ... Is there a way to tell more about what the ragel machine has been doing and why li_open was called this last time?
found the bug in a lighthouse ticket from last February: http://jgarber.lighthouseapp.com/projects/13054/tickets/183-three-newlines-after-a-list-at-end-of-file-make-dangling-li
Yep, it''s an ugly one. I''ve tried to figure it out last year, but have never been able to. Ragel is just too much of a black box?at least to me it is. I''ve printed state machine diagrams that take up the whole floor trying to figure out this stuff. It''s impossible! I don''t think crazy edge cases like this one will get resolved until I switch RedCloth to a different architecture that has a little more transparency. Jason On Fri, Jan 14, 2011 at 1:21 AM, Stephen Bannasch < stephen.bannasch at deanbrook.org> wrote:> found the bug in a lighthouse ticket from last February: > > > http://jgarber.lighthouseapp.com/projects/13054/tickets/183-three-newlines-after-a-list-at-end-of-file-make-dangling-li > > _______________________________________________ > Redcloth-upwards mailing list > Redcloth-upwards at rubyforge.org > http://rubyforge.org/mailman/listinfo/redcloth-upwards >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/redcloth-upwards/attachments/20110114/bf6a25f8/attachment.html>
>Yep, it''s an ugly one. I''ve tried to figure it out last year, but have never been able to. Ragel isjust too much of a black box-at least to me it is. I''ve printed state machine diagrams that take up the whole floor trying to figure out this stuff. It''s impossible! > >I don''t think crazy edge cases like this one will get resolved until I switch RedCloth to a different architecture that has a little more transparency.Fixed in new pull request: https://github.com/jgarber/redcloth/pull/2 Ticket updated also.
Thanks so much for the patch! Applied. On Fri, Jan 14, 2011 at 11:54 AM, Stephen Bannasch < stephen.bannasch at deanbrook.org> wrote:> >Yep, it''s an ugly one. I''ve tried to figure it out last year, but have > never been able to. Ragel isjust too much of a black box-at least to me it > is. I''ve printed state machine diagrams that take up the whole floor trying > to figure out this stuff. It''s impossible! > > > >I don''t think crazy edge cases like this one will get resolved until I > switch RedCloth to a different architecture that has a little more > transparency. > > Fixed in new pull request: https://github.com/jgarber/redcloth/pull/2 > > Ticket updated also. > _______________________________________________ > Redcloth-upwards mailing list > Redcloth-upwards at rubyforge.org > http://rubyforge.org/mailman/listinfo/redcloth-upwards >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/redcloth-upwards/attachments/20110123/b762f514/attachment.html>
At 6:41 PM -0600 1/23/11, Jason Garber wrote:>Thanks so much for the patch! Applied.Your welcome! I was surprised that I was able to find the place to fix the bug as quickly as I did -- I didn''t want to spend much time and wasn''t sure ragel would come back to me fast enough. I was very glad there are a bunch of tests -- because I sure couldn''t easily tell that my change wouldn''t break something else important. What''s your plan for releasing 4.2.4? In terms of changes in operation since 4.2.3 it looks like just: Fix double-push attempt. https://github.com/jgarber/redcloth/commit/25360cdc922bea6f5f956ec3ddfbfc6c1ec54018 and fix dangling <li> https://github.com/jgarber/redcloth/commit/ade0bfd1a44dedc2498112a069d0c77505ceb5e4 Some other changes to the build process.
I was waiting to release until multi-byte characters in Java were fixed, but seems like that''s not likely to happen anytime soon. I''ll go ahead and release. Thanks for the nudge. Jason On Sun, Jan 23, 2011 at 10:00 PM, Stephen Bannasch <stephen.bannasch at deanbrook.org> wrote:> At 6:41 PM -0600 1/23/11, Jason Garber wrote: >>Thanks so much for the patch! Applied. > > Your welcome! > > I was surprised that I was able to find the place to fix the bug as quickly as I did -- I didn''t want to spend much time and wasn''t sure ragel would come back to me fast enough. > > I was very glad there are a bunch of tests -- because I sure couldn''t easily tell that my change wouldn''t break something else important. > > What''s your plan for releasing 4.2.4? > > In terms of changes in operation since 4.2.3 it looks like just: > > ?Fix double-push attempt. > ?https://github.com/jgarber/redcloth/commit/25360cdc922bea6f5f956ec3ddfbfc6c1ec54018 > > and > > ?fix dangling <li> > ?https://github.com/jgarber/redcloth/commit/ade0bfd1a44dedc2498112a069d0c77505ceb5e4 > > Some other changes to the build process. > _______________________________________________ > Redcloth-upwards mailing list > Redcloth-upwards at rubyforge.org > http://rubyforge.org/mailman/listinfo/redcloth-upwards >
At 7:01 AM -0600 1/24/11, Jason Garber wrote:>I was waiting to release until multi-byte characters in Java were >fixed, but seems like that''s not likely to happen anytime soon. I''ll >go ahead and release. Thanks for the nudge.$ gem search RedCloth -r *** REMOTE GEMS *** RedCloth (4.2.3 ruby universal-java x86-mswin32-60) 4.2.4 nudge ;-)
Ta da! https://rubygems.org/gems/RedCloth/versions/4.2.4 Thanks for the nudges, Jason On Sun, Feb 6, 2011 at 4:05 PM, Stephen Bannasch < stephen.bannasch at deanbrook.org> wrote:> At 7:01 AM -0600 1/24/11, Jason Garber wrote: > >I was waiting to release until multi-byte characters in Java were > >fixed, but seems like that''s not likely to happen anytime soon. I''ll > >go ahead and release. Thanks for the nudge. > > > $ gem search RedCloth -r > > *** REMOTE GEMS *** > > RedCloth (4.2.3 ruby universal-java x86-mswin32-60) > > 4.2.4 nudge ;-) > _______________________________________________ > Redcloth-upwards mailing list > Redcloth-upwards at rubyforge.org > http://rubyforge.org/mailman/listinfo/redcloth-upwards >-------------- next part -------------- An HTML attachment was scrubbed... URL: <http://rubyforge.org/pipermail/redcloth-upwards/attachments/20110207/d71980b8/attachment.html>