You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@velocity.apache.org by "Henning P. Schmiedehausen" <hp...@intermeta.de> on 2005/10/29 13:44:06 UTC
Separating out the generated parser code
Hi,
I toyed a bit with pulling out the JavaCC generated code into its own
compilation tree. I got a working version, but it comes with a price,
so I want do discuss this first before I go ahead and actually
implement it.
The following files will be generated by JJTree:
CharStream.java
JJTParserState.java (JJTree)
Node.java (JJTree)
Parser.java
ParserConstants.java
ParserException.java
ParserTokenManager.java
ParserTreeConstants.java (JJTree)
ParserVisitor.java (JJTree)
Token.java
TokenMgrError.java
The following problems exist:
- While it is possible to use different packages for JJTree and JavaCC, it
actually does not work. Parser.java will use JJTParserState extensively
and that class is package protected. And there seems to be no way around
this.
That also puts the ParserVisitor in the ...parser package. And this is a
problem. In the currently (modified) grammar, the ParserVisitor is part
of the node package. And it must be there because I can't seem to be able
to put an "import org.apache.velocity.parser.node.*" statement into this
autogenerated file and I don't seem to be able to split the JJTParserState
into ...parser and the ParserVisitor into ...parser.node
So in the end, the parser must be in one package. And this means, everything
from the parser.node package (all the AST nodes) would have to go into this
package, too. This is a matter of a simple Refactoring (which eclipse does
happily for me), but it is quite a change (though it doesn't seem to be
really user visible) which I want to discuss first.
- The current "Node.java" interface is an extended version of the original
Node. If we want to keep the auto-generated files clean, we must just rename
it to VelocityNode and extend the autogenerated Node interface.
We also lose jjtOpen, jjtClose, jjtSetParent, jjtGetParent, jjtAddChild
jjtGetChild, jjtGetNumChildren, jjtAccept from this VelocityNode
interface as they are defined in the generated Node.
- in SimpleNode, the jjtGetParent changes its Signature from
VelocityNode to Node, Same goes for jjtSetParent(Node n), jjtGetChild(int i)
and jjtAddChild(Node n, int i)
now there are a bazillion errors with jjtGetChild and returning the
wrong type for all the parser nodes. So we add a getChild(int i) method to
SimpleNode and VelocityNode, which does the same as jjtGetChild(int i) but
returns a VelocityNode instead of a Node. Replace all jjtGetChild()
occurences in the VelocityCode (can also be done with Eclipse
Refactoring).
- As the ParserVisitor class fell behind the changes in the Parser because
it was not autogenerated, the visitors in parser.visitor now have a number
of missing methods. Add visit() for ASTEscapedDirective, ASTEscape, ASTMap,
ASTIntegerRange and ASTStop to NodeViewVisitor and BaseVisitor. This is
IMHO a logic bug BTW which would've been caught by ParserVisitor being
autogenerated instead of manually.
- Adding code to the ParseException wasn't really a good idea. Factor the
template code out into an TemplateParseException, also cleaning up the
code a bit. There are some hints in the JavaCC manual about customizing the
error messages, I will dig into this.
That's it. Lots of code shuffling, not actually many changes. In the end,
IMHO we will benefit from these changes in the long run.
The resulting engine does pass all the tests so at least it does not
crash and burn immediately. Some of that shuffling (the ParseException
bit) I will put in anyway; while the change to display the
templateName is nice, it is not actually intuitive or really readable
code. :-)
If no-one objects loudly, I will apply these changes tomorrow.
Best regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
4 - 8 - 15 - 16 - 23 - 42
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Separating out the generated parser code
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
"Will Glass-Husain" <wg...@forio.com> writes:
>> Ok. That pretty much vetoes seperating out the parser code. Then I will
>> start to fix up the maven build and the docs.
>sorry... it was a good idea.
No problem. IMHO it still is a good idea and we should simply target
it for 2.0. As you said, the "custom directive" stuff is
"semi-official" with some docs. We can simply make it official for
2.0. And who knows? Maybe the JavaCC folks get their act together and
allow more customization of the generated code. They could use
e.g. Velocity to build it. ;-)
Best regards
Henning
>----- Original Message -----
>From: "Henning P. Schmiedehausen" <hp...@intermeta.de>
>Newsgroups: hometree.jakarta.velocity.dev
>To: <ve...@jakarta.apache.org>
>Sent: Sunday, October 30, 2005 4:04 AM
>Subject: Re: Separating out the generated parser code
>> "Will Glass-Husain" <wg...@forio.com> writes:
>>
>>>Hi,
>>
>>>I'm reluctant to endorse changes to our public API for versions 1.x.
>>>Creating custom directives is obscure, but I'm sure there's others who do
>>>this. (One of the articles linked to from our web site has an example of
>>>a
>>>custom directive, for instance.) I think we should continue to have a
>>>"drop-in upgrade" objective for all publically documented features.
>>
>> Ok. That pretty much vetoes seperating out the parser code. Then I will
>> start to fix up the maven build and the docs.
>>
>>>Also - I wanted to check. Are we keeping the capability introduced in the
>>>recent patch of tracking the template name in parse errors? I like the
>>>sound of your proposed changes on this but want to confirm the errors will
>>>continue to have the template name.
>>
>> Yes, they should have. Even better, the MacroParseException will now
>> behave similar to the introduced changes in ParseException. If they
>> don't, then I messed up and it's a bug. :-)
>>
>> Best regards
>> Henning
>>
>> --
>> Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
>> hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
>>
>> RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
>> Linux, Java, perl, Solaris -- Consulting, Training, Development
>>
>> 4 - 8 - 15 - 16 - 23 - 42
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>>
>---------------------------------------------------------------------
>To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
>For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
4 - 8 - 15 - 16 - 23 - 42
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Separating out the generated parser code
Posted by Will Glass-Husain <wg...@forio.com>.
> Ok. That pretty much vetoes seperating out the parser code. Then I will
> start to fix up the maven build and the docs.
sorry... it was a good idea.
----- Original Message -----
From: "Henning P. Schmiedehausen" <hp...@intermeta.de>
Newsgroups: hometree.jakarta.velocity.dev
To: <ve...@jakarta.apache.org>
Sent: Sunday, October 30, 2005 4:04 AM
Subject: Re: Separating out the generated parser code
> "Will Glass-Husain" <wg...@forio.com> writes:
>
>>Hi,
>
>>I'm reluctant to endorse changes to our public API for versions 1.x.
>>Creating custom directives is obscure, but I'm sure there's others who do
>>this. (One of the articles linked to from our web site has an example of
>>a
>>custom directive, for instance.) I think we should continue to have a
>>"drop-in upgrade" objective for all publically documented features.
>
> Ok. That pretty much vetoes seperating out the parser code. Then I will
> start to fix up the maven build and the docs.
>
>>Also - I wanted to check. Are we keeping the capability introduced in the
>>recent patch of tracking the template name in parse errors? I like the
>>sound of your proposed changes on this but want to confirm the errors will
>>continue to have the template name.
>
> Yes, they should have. Even better, the MacroParseException will now
> behave similar to the introduced changes in ParseException. If they
> don't, then I messed up and it's a bug. :-)
>
> Best regards
> Henning
>
> --
> Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
> hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
>
> RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
> Linux, Java, perl, Solaris -- Consulting, Training, Development
>
> 4 - 8 - 15 - 16 - 23 - 42
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Separating out the generated parser code
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
"Will Glass-Husain" <wg...@forio.com> writes:
>Hi,
>I'm reluctant to endorse changes to our public API for versions 1.x.
>Creating custom directives is obscure, but I'm sure there's others who do
>this. (One of the articles linked to from our web site has an example of a
>custom directive, for instance.) I think we should continue to have a
>"drop-in upgrade" objective for all publically documented features.
Ok. That pretty much vetoes seperating out the parser code. Then I will
start to fix up the maven build and the docs.
>Also - I wanted to check. Are we keeping the capability introduced in the
>recent patch of tracking the template name in parse errors? I like the
>sound of your proposed changes on this but want to confirm the errors will
>continue to have the template name.
Yes, they should have. Even better, the MacroParseException will now
behave similar to the introduced changes in ParseException. If they
don't, then I messed up and it's a bug. :-)
Best regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
4 - 8 - 15 - 16 - 23 - 42
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Separating out the generated parser code
Posted by Will Glass-Husain <wg...@forio.com>.
Hi,
I'm reluctant to endorse changes to our public API for versions 1.x.
Creating custom directives is obscure, but I'm sure there's others who do
this. (One of the articles linked to from our web site has an example of a
custom directive, for instance.) I think we should continue to have a
"drop-in upgrade" objective for all publically documented features.
Also - I wanted to check. Are we keeping the capability introduced in the
recent patch of tracking the template name in parse errors? I like the
sound of your proposed changes on this but want to confirm the errors will
continue to have the template name.
Best,
WILL
----- Original Message -----
From: "Henning P. Schmiedehausen" <hp...@intermeta.de>
Newsgroups: hometree.jakarta.velocity.dev
To: <ve...@jakarta.apache.org>
Sent: Saturday, October 29, 2005 12:20 PM
Subject: Re: Separating out the generated parser code
> or be a real improvement (line/column/template for exceptions). As I
> know only of Will to actually use custom directives, I want to check
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Separating out the generated parser code
Posted by "Henning P. Schmiedehausen" <hp...@intermeta.de>.
Nathan Bubna <nb...@gmail.com> writes:
>is this going to cause any problems for people who write their own
>directives? and if so, can we provide a deprecation path for them?
Actually yes, it is. The problem would be that currently
o.a.v.r.parser.node.Node is part of the visible interface.
In the changes that I've already put in, I worked around this by
keeping the name and package and just extending o.a.v.r.parser.Node
This is simply not possible when fully separating the code
out. Because the whole parser code _must_ go into a single
package. And because it does so and because <this package>.Node will
be auto-generated, the currently o.a.v.r.parser.node.Node must be
renamed. So this will break custom directives.
I don't even see a possible deprecation path, sorry. I'm very open for
suggestions here.
[...]
>> - The current "Node.java" interface is an extended version of the original
>> Node. If we want to keep the auto-generated files clean, we must just rename
>> it to VelocityNode and extend the autogenerated Node interface.
>>
>> We also lose jjtOpen, jjtClose, jjtSetParent, jjtGetParent, jjtAddChild
>> jjtGetChild, jjtGetNumChildren, jjtAccept from this VelocityNode
>> interface as they are defined in the generated Node.
>>
>> - in SimpleNode, the jjtGetParent changes its Signature from
>> VelocityNode to Node, Same goes for jjtSetParent(Node n), jjtGetChild(int i)
>> and jjtAddChild(Node n, int i)
>>
>> now there are a bazillion errors with jjtGetChild and returning the
>> wrong type for all the parser nodes. So we add a getChild(int i) method to
>> SimpleNode and VelocityNode, which does the same as jjtGetChild(int i) but
>> returns a VelocityNode instead of a Node. Replace all jjtGetChild()
>> occurences in the VelocityCode (can also be done with Eclipse
>> Refactoring).
This part is already in, but because the packages are different, there
are no user visible changes.
>> - As the ParserVisitor class fell behind the changes in the Parser because
>> it was not autogenerated, the visitors in parser.visitor now have a number
>> of missing methods. Add visit() for ASTEscapedDirective, ASTEscape, ASTMap,
>> ASTIntegerRange and ASTStop to NodeViewVisitor and BaseVisitor. This is
>> IMHO a logic bug BTW which would've been caught by ParserVisitor being
>> autogenerated instead of manually.
I've updated ParserVisitor to match the auto-generated class but kept it in the
node package. This also updates the visitor classes.
>> - Adding code to the ParseException wasn't really a good idea. Factor the
>> template code out into an TemplateParseException, also cleaning up the
>> code a bit. There are some hints in the JavaCC manual about customizing the
>> error messages, I will dig into this.
I've cleaned that bit up too, in the progress I've broken and updated
the MacroParseException :-)
>> If no-one objects loudly, I will apply these changes tomorrow.
After looking into the directive problem, I will not apply more
changes (the stuff that is in there should be either not user visible
or be a real improvement (line/column/template for exceptions). As I
know only of Will to actually use custom directives, I want to check
with him first.
Best regards
Henning
--
Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
Linux, Java, perl, Solaris -- Consulting, Training, Development
4 - 8 - 15 - 16 - 23 - 42
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
Re: Separating out the generated parser code
Posted by Nathan Bubna <nb...@gmail.com>.
is this going to cause any problems for people who write their own
directives? and if so, can we provide a deprecation path for them?
On 10/29/05, Henning P. Schmiedehausen <hp...@intermeta.de> wrote:
> Hi,
>
> I toyed a bit with pulling out the JavaCC generated code into its own
> compilation tree. I got a working version, but it comes with a price,
> so I want do discuss this first before I go ahead and actually
> implement it.
>
> The following files will be generated by JJTree:
>
> CharStream.java
> JJTParserState.java (JJTree)
> Node.java (JJTree)
> Parser.java
> ParserConstants.java
> ParserException.java
> ParserTokenManager.java
> ParserTreeConstants.java (JJTree)
> ParserVisitor.java (JJTree)
> Token.java
> TokenMgrError.java
>
> The following problems exist:
>
> - While it is possible to use different packages for JJTree and JavaCC, it
> actually does not work. Parser.java will use JJTParserState extensively
> and that class is package protected. And there seems to be no way around
> this.
>
> That also puts the ParserVisitor in the ...parser package. And this is a
> problem. In the currently (modified) grammar, the ParserVisitor is part
> of the node package. And it must be there because I can't seem to be able
> to put an "import org.apache.velocity.parser.node.*" statement into this
> autogenerated file and I don't seem to be able to split the JJTParserState
> into ...parser and the ParserVisitor into ...parser.node
>
> So in the end, the parser must be in one package. And this means, everything
> from the parser.node package (all the AST nodes) would have to go into this
> package, too. This is a matter of a simple Refactoring (which eclipse does
> happily for me), but it is quite a change (though it doesn't seem to be
> really user visible) which I want to discuss first.
>
> - The current "Node.java" interface is an extended version of the original
> Node. If we want to keep the auto-generated files clean, we must just rename
> it to VelocityNode and extend the autogenerated Node interface.
>
> We also lose jjtOpen, jjtClose, jjtSetParent, jjtGetParent, jjtAddChild
> jjtGetChild, jjtGetNumChildren, jjtAccept from this VelocityNode
> interface as they are defined in the generated Node.
>
> - in SimpleNode, the jjtGetParent changes its Signature from
> VelocityNode to Node, Same goes for jjtSetParent(Node n), jjtGetChild(int i)
> and jjtAddChild(Node n, int i)
>
> now there are a bazillion errors with jjtGetChild and returning the
> wrong type for all the parser nodes. So we add a getChild(int i) method to
> SimpleNode and VelocityNode, which does the same as jjtGetChild(int i) but
> returns a VelocityNode instead of a Node. Replace all jjtGetChild()
> occurences in the VelocityCode (can also be done with Eclipse
> Refactoring).
>
> - As the ParserVisitor class fell behind the changes in the Parser because
> it was not autogenerated, the visitors in parser.visitor now have a number
> of missing methods. Add visit() for ASTEscapedDirective, ASTEscape, ASTMap,
> ASTIntegerRange and ASTStop to NodeViewVisitor and BaseVisitor. This is
> IMHO a logic bug BTW which would've been caught by ParserVisitor being
> autogenerated instead of manually.
>
> - Adding code to the ParseException wasn't really a good idea. Factor the
> template code out into an TemplateParseException, also cleaning up the
> code a bit. There are some hints in the JavaCC manual about customizing the
> error messages, I will dig into this.
>
> That's it. Lots of code shuffling, not actually many changes. In the end,
> IMHO we will benefit from these changes in the long run.
>
> The resulting engine does pass all the tests so at least it does not
> crash and burn immediately. Some of that shuffling (the ParseException
> bit) I will put in anyway; while the change to display the
> templateName is nice, it is not actually intuitive or really readable
> code. :-)
>
> If no-one objects loudly, I will apply these changes tomorrow.
>
> Best regards
> Henning
>
> --
> Dipl.-Inf. (Univ.) Henning P. Schmiedehausen INTERMETA GmbH
> hps@intermeta.de +49 9131 50 654 0 http://www.intermeta.de/
>
> RedHat Certified Engineer -- Jakarta Turbine Development -- hero for hire
> Linux, Java, perl, Solaris -- Consulting, Training, Development
>
> 4 - 8 - 15 - 16 - 23 - 42
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
> For additional commands, e-mail: velocity-dev-help@jakarta.apache.org
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: velocity-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: velocity-dev-help@jakarta.apache.org