XWikiParser - Newline at the end of a group is not removed when the element before is a macro

[GoogleCodeExporter]

See http://jira.xwiki.org/jira/browse/XWIKI-5060

Original issue reported on code.google.com by thomas.m...@gmail.com on 29 Mar 2010 at 9:23

[GoogleCodeExporter]

New lines need to be treated carefully, as they should not generate new line 
events
unless there is some elements following the new line.  

A macro may or may not be followed by a new line character.  This should be 
treated
the same way as new lines within paragraphs.  So, I added a "linesMaybeEmpty"
production that should fix this particular problem the same way as it is done 
for
ordinary paragraphs.

Index: src/test/java/org/wikimodel/wem/test/XWiki20ParserTest.java
===================================================================
--- src/test/java/org/wikimodel/wem/test/XWiki20ParserTest.java (revision 462)
+++ src/test/java/org/wikimodel/wem/test/XWiki20ParserTest.java (arbetskopia)
@@ -509,7 +509,7 @@

         test(
             "{{macro/}}\n",
-            "<p><span class='wikimodel-macro' macroName='macro'/>\n</p>");
+            "<pre class='wikimodel-macro' macroName='macro'/>");

         test(
             "{{macro/}}\n\n{{macro/}}",
Index: src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj
===================================================================
--- 
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj    (revision 
462)
+++ 
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj    (arbetskopi
a)
@@ -642,11 +643,10 @@
          processMacro(s, c, inline);
       }
    }
-   ( LOOKAHEAD(1) newLine() )?
-   ( LOOKAHEAD(1) lines() )?
-       {
-           fContext.endParagraph();
-       }
+   linesMaybeEmpty()
+   {
+       fContext.endParagraph();
+   }
 }

 void list():
@@ -883,6 +883,20 @@
     )*
 }

+void linesMaybeEmpty():
+{
+}
+{
+    [LOOKAHEAD(1) line()] (LOOKAHEAD(1) <NL>)?
+    (
+       LOOKAHEAD(1) 
+       (
+          ({fContext.onNewLine();}line()) 
+          (LOOKAHEAD(1) <NL>)?
+       )
+    )*
+}
+

Original comment by AndreasZ...@gmail.com on 1 Apr 2010 at 2:44

[GoogleCodeExporter]

Note that there is probably the same issue with verbatim blocks since they have 
the
same inline/standalone states.

Original comment by thomas.m...@gmail.com on 1 Apr 2010 at 2:52

[GoogleCodeExporter]

Indeed!  I now realize that it is up to the listener to decide whether the 
macro or
verbatim turns into an inlined element when its followed by something.  Nothing 
in
the parser indicates that that would happen.  This had me a bit confused.  

Index: src/test/java/org/wikimodel/wem/test/XWiki20ParserTest.java
===================================================================
--- src/test/java/org/wikimodel/wem/test/XWiki20ParserTest.java (revision 462)
+++ src/test/java/org/wikimodel/wem/test/XWiki20ParserTest.java (arbetskopia)
@@ -941,11 +942,11 @@
         test(
             "some text {{{}}}\n\n{{{}}}",
             "<p>some text <tt class=\"wikimodel-verbatim\"></tt></p>\n<pre></pre>");
-        test("{{{}}}\n{{{}}}\n{{{}}}", 
"<pre></pre>\n<pre></pre>\n<pre></pre>");
+        test("{{{}}}\n{{{}}}\n{{{}}}", "<p><tt
class=\"wikimodel-verbatim\"></tt>\n<tt class=\"wikimodel-verbatim\"></tt>\n<tt
class=\"wikimodel-verbatim\"></tt></p>");
         test(
             "{{{}}}\n\n{{{}}}\n\n{{{}}}\n\n{{{}}}",
             "<pre></pre>\n<pre></pre>\n<pre></pre>\n<pre></pre>");
-        test("{{{}}}\nsome text", "<pre></pre>\n<p>some text</p>");
+        test("{{{}}}\nsome text", "<p><tt 
class=\"wikimodel-verbatim\"></tt>\nsome
text</p>");

         test(
             "some text {{{}}}\nsome text",
Index: src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj
===================================================================
--- 
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj    (revision 
462)
+++ 
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj    (arbetskopi
a)
@@ -776,7 +776,10 @@
             }
         }
     }
-    (LOOKAHEAD(1) line())?
+   linesMaybeEmpty()
+   {
+       fContext.endParagraph();
+   }
 }

 void horline():

Original comment by AndreasZ...@gmail.com on 1 Apr 2010 at 5:20

[GoogleCodeExporter]

Both macro and verbatim fixes as one patch attached.

Original comment by AndreasZ...@gmail.com on 1 Apr 2010 at 5:26

Attachments:

• patch

[GoogleCodeExporter]

Your patch seems to work well, just don't forget to add related unit test 
(macro and
verbatim at the end of a group) before committing, never too much 
non-regression tests ;)

Original comment by thomas.m...@gmail.com on 7 Apr 2010 at 2:16

[GoogleCodeExporter]

Sorry about the delay!  I have not been working for a few days (for various 
reasons)
and I didn't realize that I was expected to commit this.  

Now I have also managed to input the wrong password when trying to commit (I was
cutting and pasting the generated password, so I don't know how this could 
happen),
so it seems that my IP-number is blocked from committing for some unknown time 
(I'm
guessing about 24 hours).  I'll commit as soon as this block has been lifted.

Original comment by AndreasZ...@gmail.com on 8 Apr 2010 at 7:59

[GoogleCodeExporter]

Don't worry, there is no rush, i was just not sure if you were expecting a 
review for
your patch before committing or not ;)

Original comment by thomas.m...@gmail.com on 8 Apr 2010 at 8:11

[GoogleCodeExporter]

Committed the patch and some unit test.

Original comment by AndreasZ...@gmail.com on 8 Apr 2010 at 8:46

• Changed state: Fixed

[GoogleCodeExporter]

Ok cool, just one thing. When you fix a wikimodel bug related to XWiki, you 
should
indicate it on XWiki side so that we know that we should upgrade wikimodel 
version.
For this use case for example you can comment on
http://jira.xwiki.org/jira/browse/XWIKI-5060

Original comment by thomas.m...@gmail.com on 8 Apr 2010 at 8:55

[GoogleCodeExporter]

Last thing: you forgot to update 2.1 syntax. Basically before committing you 
should
check that "mvn clean install" works. This also check that a change you would 
have
made to comment code for example does not break other syntaxes unit tests.

Original comment by thomas.m...@gmail.com on 8 Apr 2010 at 9:00

[GoogleCodeExporter]

Sorry, I'll add it to 2.1 as well.

Original comment by AndreasZ...@gmail.com on 8 Apr 2010 at 9:28