Closed GoogleCodeExporter closed 8 years ago
I'm appending a new version of the parser that also passes the unit tests when
compiling xwiki core (except for the dangling )))-token, of course) and seem to
work
fine on an XWiki 2.1 installation.
Original comment by AndreasZ...@gmail.com
on 22 Jan 2010 at 2:43
Yet another version. This time everything works. Promise. ;-) The page
XWiki.XWikiSyntax renders properly, at least...
Original comment by AndreasZ...@gmail.com
on 25 Jan 2010 at 4:38
Attachments:
Hi Andreas, thanks a lot for this contribution but i can't build it: i get
[javacc] Java Compiler Compiler Version 5.0 (Parser Generator)
[javacc] (type "javacc" with no arguments for help)
[javacc] Reading from file
/media/data/projets/wikimodel/svn/org.wikimodel.wem/src/main/java/org/wikimodel/
wem/xwiki/xwiki20/javacc/XWikiScanner.jj
. . .
[javacc] org.javacc.parser.ParseException: Encountered " <IDENTIFIER> "options ""
at line 463, column 1.
[javacc] Was expecting one of:
[javacc] <STRING_LITERAL> ...
[javacc] "<" ...
[javacc]
[javacc] Detected 1 errors and 0 warnings.
when doing ant -f RebuildScanners.xml
Original comment by thomas.m...@gmail.com
on 21 Feb 2010 at 2:41
Looks like a bug in RebuildScanners.launch itself or something it does not like
in
the file since it seems to duplicate the java part in <INLINE, TABLE_CONTEXT,
HEADER_CONTEXT> context
Original comment by thomas.m...@gmail.com
on 21 Feb 2010 at 2:50
I just built it with javacc directly. But I see in the ant-file that there are
a
number of substitution rules. The "replace-default-context" and the
"replace-getters" targets should not be run. The "replace-common-tokens"
should be
ok, if you just move the #EMPTY_LINE-token outside of the <common-tokens> tags.
Original comment by AndreasZ...@gmail.com
on 22 Feb 2010 at 2:41
Ok i modified the jj file to not be impacted by RebuildScanners but there is a
failing test:
test("before inside ))) after ", "<p>before inside</p>\n" + "<p>after </p>");
produce a "ParseException: Parse error at line 1, column 15. Encountered: ))) "
(there is another one with macros but i fixed it, i did this modification after
to
provide this patch)
Original comment by thomas.m...@gmail.com
on 16 Mar 2010 at 5:19
Attachments:
> after to provide
after you provide
Original comment by thomas.m...@gmail.com
on 16 Mar 2010 at 5:20
Hi Thomas!
As I mentioned in the first comment, I changed the behavior of the dangling )))
token
as this was called for in a comment of the original grammar file. Outputting
it as
special characters seems better. Its an unusual special case and since it was
nontrivial to fix in the original grammar file I can understand why it was
solved the
way it is.
Original comment by AndreasZ...@gmail.com
on 16 Mar 2010 at 8:33
I know that (and agree with your change) but that's not the issue here, what i
get is
not a failure but an error, an exception.
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 1:34
I am sorry! You are right, the code is incorrect. Here is a patch:
--- XWikiScanner.jj.orig 2010-03-17 15:51:09.000000000 +0100
+++ XWikiScanner.jj 2010-03-17 15:54:08.000000000 +0100
@@ -495,11 +495,14 @@
int offset = s.indexOf(")))");
if (offset > 0) {
input_stream.backup(s.length() - offset);
- matchedToken.image = s.substring(0, s.length()-offset);
+ matchedToken.image = s.substring(0, offset);
matchedToken.kind = XWIKI_SPACE;
} else if (s.length() > ")))".length()) {
+ matchedToken.image = s.substring(0, ")))".length());
input_stream.backup(s.length() - ")))".length());
matchedToken.kind = XWIKI_SPECIAL_SYMBOL;
+ } else {
+ matchedToken.kind = XWIKI_SPECIAL_SYMBOL;
}
}
}
Original comment by AndreasZ...@gmail.com
on 17 Mar 2010 at 3:00
Ok thanks, i'm trying with that.
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 3:21
Does not seems to fix it. You can execute easily unit tests by building the
project
with maven (mvn install) if you want to validate your modif
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 3:31
But it passes all unit tests, both the wikimodel unit tests and the XWiki unit
tests.
I've changed group/group2.test to the following:
.#-----------------------------------------------------
.input|xwiki/2.0
.#-----------------------------------------------------
before inside ))) after
.#-----------------------------------------------------
.expect|event/1.0
.#-----------------------------------------------------
beginDocument
beginParagraph
onWord [before]
onSpace
onWord [inside]
onSpace
onSpecialSymbol [)]
onSpecialSymbol [)]
onSpecialSymbol [)]
onSpace
onWord [after]
endParagraph
endDocument
.#-----------------------------------------------------
.expect|xhtml/1.0
.#-----------------------------------------------------
<p>before inside ))) after</p>
.#-----------------------------------------------------
.expect|xwiki/2.0
.#-----------------------------------------------------
before inside ~)~)~) after
Furthermore, I've installed the fixed version on my own XWiki installation.
Have a
look here: http://kreablo.se:8080/x/bin/view/Sandbox/?language=en
I'm attaching the precise version of the file that I've built with.
Original comment by AndreasZ...@gmail.com
on 17 Mar 2010 at 5:33
Attachments:
I don't see how test("before inside ))) after ", "<p>before inside</p>\n" +
"<p>after
</p>"); in wikimodel could pass if you don't modify it, it should at least fail
since
you changed its behavior.
Testing with your last file.
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 5:50
Ok now it just fail.
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 5:56
All is working great with the last version you posted, thanks !
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 6:01
Applied patch from Andreas Jonsson, thanks again !
Original comment by thomas.m...@gmail.com
on 17 Mar 2010 at 6:09
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 6:44
Hi an Andreas,
I found one regression so I reopen this issue:
* one
* (((two)))
* three
produce two lists now
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 11:02
I committed a unit test for it. Would be great if you could fix it, I will try
to
look at it when i have some time. Would be a shame to revert your patch ;)
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 11:07
Some more informations: it breaks the list only if it's the last element of the
list item
* ((( group )))
* item
fail but
* ((( group ))) not group
* item
works well
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 11:27
(again i committed the unit test for it)
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 11:28
I'm happy that you will accept my patch. Now I could go on fixing the link
label
problem that I started to look at. :)
The embedded document inside of the list item is a bit of a special case. The
new
line must be consumed if immediately following the embedded document. Good
thing
that you found the problem.
Index: src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj
===================================================================
---
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj (revision
446)
+++
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj (working
copy)
@@ -684,7 +684,7 @@
}
(LOOKAHEAD(1) newLine() )?
( LOOKAHEAD(1)
- (embeddedDocument()|lines())
+ ((embeddedDocument() [LOOKAHEAD(1) <NL>]) | lines())
)*
{
fContext.endListItem();
Index: src/main/java/org/wikimodel/wem/xwiki/xwiki21/javacc/XWikiScanner.jj
===================================================================
---
src/main/java/org/wikimodel/wem/xwiki/xwiki21/javacc/XWikiScanner.jj (revision
446)
+++
src/main/java/org/wikimodel/wem/xwiki/xwiki21/javacc/XWikiScanner.jj (working
copy)
@@ -684,7 +684,7 @@
}
(LOOKAHEAD(1) newLine() )?
( LOOKAHEAD(1)
- (embeddedDocument()|lines())
+ ((embeddedDocument() [LOOKAHEAD(1) <NL>]) | lines())
)*
{
fContext.endListItem();
Original comment by AndreasZ...@gmail.com
on 18 Mar 2010 at 12:21
Ok thanks i'm testing your fix. BTW don't worry about 2.1 syntax for now since
it's
an exact copy of the 2.0 one.
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 12:56
Thanks, i fixes the issue.
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 1:15
Did not seen that before but javacc doe snot like one things in the jj file, i
get a
warning when i build:
[javacc] Java Compiler Compiler Version 5.0 (Parser Generator)
[javacc] (type "javacc" with no arguments for help)
[javacc] Reading from file
/media/data/projets/wikimodel/svn/org.wikimodel.wem/src/main/java/org/wikimodel/
wem/xwiki/xwiki20/javacc/XWikiScanner.jj
. . .
[javacc] Note: UNICODE_INPUT option is specified. Please make sure you create the
parser/lexer using a Reader with the correct character encoding.
[javacc] File "TokenMgrError.java" does not exist. Will create one.
[javacc] Warning: Line 274, Column 5: Regular expression can be matched by the
empty string ("") in lexical state DEFAULT. This regular expression along with
the
regular expressions at line 292, column 5 forms the cycle
[javacc] DEFAULT-->BEGINNING_OF_LINE-->BEGINNING_OF_LINE
[javacc] containing regular expressions with empty matches. This can result in an
endless loop of empty string matches.
[javacc] File "ParseException.java" does not exist. Will create one.
[javacc] File "Token.java" does not exist. Will create one.
[javacc] File "SimpleCharStream.java" does not exist. Will create one.
[javacc] Parser generated with 0 errors and 1 warnings.
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 1:16
I'm aware of this warning. It is a spurios warning which cannot be silenced.
Since
the lexical state is controlled in the java code, javacc fails to realize that
the
empty string match cannot result in an infinit loop.
The following does not yield any warning:
"" : BEGINNING_OF_LINE
But the following does:
"" {returnFromBeginningOfLine();}
javacc cannot analyze the java code, it just sees the empty string match.
Of course, this construction would be dangerous if combined with lookahead,
just as
putting back characters on the input stream is. With javacc you can choose to
control the scanner with java code or you can choose to use lookahead, but not
both
at the same time.
Original comment by AndreasZ...@gmail.com
on 18 Mar 2010 at 1:46
Ok i tough about something like that but was not sure, i will add a comment to
explain that since having a warning saying that i found an infinite loop in
your code
is always scary ;)
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 1:53
Original comment by thomas.m...@gmail.com
on 18 Mar 2010 at 1:54
I found a serious error. The lexical state is not switched after a heading
that has
content additional content on the same line. I cannot believe that this error
slipped
through as I thought I was beeing particularly careful to inspect all cases
where the
lexical state must be switched. :(
Here is a patch:
Index: src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj
===================================================================
---
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj (revision
446)
+++
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj (arbetskopi
a)
@@ -187,7 +187,7 @@
*
* Except for state transitions to BEGINNING_OF_LINE and to
* special states, we need to control the state transitions in
- * java code. For this control the state we keep three variables:
+ * java code. For this control the state we keep two variables:
*
* * preceedingSpecialState
* * stateStack
@@ -301,7 +301,8 @@
<HEADER_CONTEXT> TOKEN:
{
<HEADER_END: (<SPACE>)* ("=")+ <NEW_LINE> > {clearBlockState();} :
BEGINNING_OF_LINE
- | <HEADER_END_INLINE: (<SPACE>)* ("=")+> {
+ | <HEADER_END_INLINE: (<SPACE>)* ("=")+> {
+ clearBlockState();
matchedToken.kind = HEADER_END;
} : INLINE
| <NEW_HEADER_BEGIN: <NEW_LINE> <HEADER_BEGIN_PATTERN>> { matchedToken.kind =
HEADER_BEGIN; }
Original comment by AndreasZ...@gmail.com
on 19 Mar 2010 at 11:58
Thanks, i'm applying it, looks like google code does not send mails for
comments on
closed issue...
Original comment by thomas.m...@gmail.com
on 19 Mar 2010 at 1:56
Original comment by thomas.m...@gmail.com
on 19 Mar 2010 at 2:26
New regression:
>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
(% param="value" %)
{{macro}}conten{{/macro}}
after
<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<
produce a standalone macro followed by a paragraph with custom parameters
followed by
a paragraph with "after" in it where it should be an inline macro inside a
paragraph
with custom parameters followed by the "after" paragraph.
I just committed a unit test for it.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 9:47
Another one which is certainly related of the cause of the other: (%
param='value'
%)\n does not produce an empty paragraph as it should.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 10:08
Ok i understand what is the issue but not sure how to fix it, problem is that
custom
parameters are saved for the following block but without theses parameters
impossible
to match an empty paragraph...
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 10:25
There is a general issue with paragraph parameters and macros. Added some more
unit
tests.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 10:39
I guess there is the exact same issue with verbatim
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 10:58
The block parameters are stored when parsing them and the consumed by the
following
block.
The macro block currently does not consume the parameters, hence the problem.
This
should not occur for verbatim blocks, as there the parameters are consumed.
But the macro block seems to be treated somewhat differently from other
elements in
that the parameters are not passed directly to the onMacro method. The
parameters
passed to onMacro are the macro parameters. So is it the case that a macro
block
should always be inserted in a paragraph that takes the wiki parameters?
As of parameters at the end of the document, since no block element is
following the
parameters, they are not consumed. A fix would be to check if there are any
remaining parameters at the end of the document and insert an empty paragraph
in that
case.
Original comment by AndreasZ...@gmail.com
on 25 Mar 2010 at 11:28
The below patch fixes the problems. But I now realize what you mean about
verbatim
and you may be right that the same problem occurs there. I'll look into it.
Index: src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj
===================================================================
---
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj (revision
455)
+++
src/main/java/org/wikimodel/wem/xwiki/xwiki20/javacc/XWikiScanner.jj (working
copy)
@@ -69,6 +69,14 @@
return params;
}
+ protected void consumeRemainingParameters() {
+ if (wikiParameters != WikiParameters.EMPTY) {
+ fContext.beginParagraph(wikiParameters);
+ fContext.endParagraph();
+ wikiParameters = WikiParameters.EMPTY;
+ }
+ }
+
private int emptyLinesCount = 0;
private void endBlock() {
@@ -93,9 +101,10 @@
}
private void endDocument() {
- if (emptyLinesCount > 1) {
- fContext.onEmptyLines(emptyLinesCount);
- }
+ consumeRemainingParameters();
+ if (emptyLinesCount > 1) {
+ fContext.onEmptyLines(emptyLinesCount);
+ }
}
private void processMacro(String start, String content, boolean inline) {
@@ -114,7 +123,13 @@
if (inline) {
fContext.onMacro(name, params, content, inline);
} else {
- fContext.onMacro(name, params, content);
+ WikiParameters wikiParams = consumeWikiParameters();
+ if (wikiParams != WikiParameters.EMPTY) {
+ fContext.beginParagraph(wikiParams);
+ fContext.onMacro(name, params, content, true);
+ } else {
+ fContext.onMacro(name, params, content);
+ }
}
}
@@ -649,7 +664,11 @@
processMacro(s, c, inline);
}
}
- (LOOKAHEAD(1) line())?
+ ( LOOKAHEAD(1) newLine() )?
+ ( LOOKAHEAD(1) lines() )?
+ {
+ fContext.endParagraph();
+ }
}
void list():
Original comment by AndreasZ...@gmail.com
on 25 Mar 2010 at 12:15
Th old behavior was that custom parameters was starting a paragraph except for
some
special blocks (like list, tables, etc...) that consume parameters. So the
macro like
any inline element was part of the paragraph.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 12:31
I think the parser should be refactored to something like:
header(parameters)
|
list(parameters)
|
quot(parameters)
|
horline(parameters)
|
table(parameters)
|
paragraph(parameters)
after a blockParameters() has been found. That way macro and verbatim would be
part
of paragraph(parameters) and empty paragraph would be supported properly. It
also
makes storing the parameters in a variable useless since we give it directly to
the
following block.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 12:41
I'm trying something
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 12:48
But the parameters must be passed also to all inlined elements, so I think it
would
just be syntactically clumsy to make it an explicit argument.
Also, since macros and verbatim in block modes are freestanding elements, they
cannot
be made part of paragraph.
Original comment by AndreasZ...@gmail.com
on 25 Mar 2010 at 12:53
Macros are handled inconsistently from other elements in that it is currently
impossible to pass custom parameters to a block macro. If you pass parameters
it
will turn into an inlined macro. To resolve this the listener API must be
changed to
the following:
void onMacroBlock(String macroName, WikiParameters macroParams, String content,
WikiParameters params);
The same problems will not occur for verbatim blocks as we do not need to all
of a
sudden turn it into an inlinde element.
Original comment by AndreasZ...@gmail.com
on 25 Mar 2010 at 1:03
For now my only concern is to not break xwiki/2.0 syntax and in xwiki/2.0 syntax
macro does not support custom parameters so I'm fixing the parser for it.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 1:05
Patch applied. Not sure if it was on purpose or not but you also fixed a bug: i
created http://jira.xwiki.org/jira/browse/XWIKI-5050 to describe it.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 2:34
I noticed that my patch seems to break more than it fixes when running the
XWiki unit
tests. I need to think it through in depth to make sure it works in all cases,
but I
don't have the time right now. I'll get back on this later
Original comment by AndreasZ...@gmail.com
on 25 Mar 2010 at 3:17
[deleted comment]
I already applied your patch and as i said it does not break anything, it was
wrong
tests.
Original comment by thomas.m...@gmail.com
on 25 Mar 2010 at 3:24
OK, great! That was mostly by accident. :)
Original comment by AndreasZ...@gmail.com
on 25 Mar 2010 at 4:43
Original issue reported on code.google.com by
AndreasZ...@gmail.com
on 18 Jan 2010 at 6:33