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 <[EMAIL PROTECTED]> 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 > [EMAIL PROTECTED] +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: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
