I don't like the idea of ClassCompiler.updateSchema not calling its super. This makes me think there is something wrong with our modularization. Maybe you should leave the super call and conditionalize the code in ViewCompiler to not run on subclasses?
tagOrClassName and getClassModel together duplicate (but with less error checking, and I think with a bug) getParentClassModel. Consider a class definition that does not specify extends because it extends the default class, view. tagOrClassName will say it's a 'class' when it's really a 'view'. I think getClassModel should be replaced by getParentClassModel. I'm confused about the use of tagOrClassName in checkRequiredAttributes. If I am defining a new class, is it checking that my definition has the required attributes of a <class> or of my superclass? At this point, I think I hit some merge errors. More when you have an updated review. On 2009-11-20, at 10:23, Henry Minsky wrote: > Change 20091120-hqm-i by [email protected] on 2009-11-20 08:58:31 EST > in /Users/hqm/openlaszlo/trunk > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: support mixins on instances > > New Features: > > Bugs Fixed: LPP-8602 Allow with="" on instances declarations > > Technical Reviewer: ptw > QA Reviewer: max > Doc Reviewer: (pending) > > Documentation: > > Release Notes: > > mixins are supported now on instances as well as classes, e.g., > > <mixin name="textmixin"> > <attribute name="foo" value="bar" type="text"/> > </mixin> > > <text name="mixinstance" with="textmixin"> > <attribute name="text" value="${this.foo}"/> > </text> > > Overview: > > > Details: > > > ClassCompiler.java: remove call to super.updateSchema, since we made > ViewCompiler's updateSchema now do some things that only apply to > instances. > > schema/lfc-undeclared.lzx: Moved the "with" atribute down from <class> to > <node> > beause instances can now have mixins > > ViewCompiler.java: add updateSchema method, so that instances with mixins will > call the ClassModel machinery to add the needed interstitial classes to the > app. > This is done by rewriting the instance as a <anonymous extends="tagclass"> > instance, > and then ClassModel and NodeModel have been modified to know how to deal with > these > 'class-like instances' > > Also removed call to the class inlining code that is no longer used in > the compiler. > > > ToplevelCompiler.java: use generalized 'tagname' accessor to get the > classname of an instance, since an element may be an anonymous instance > class. > > > ViewSchema.java: add an explicit arg to say if we're defining a public class > or a > private (anonymous instance) one > > NodeModel.java: Since instance classes may now be given a 'anonymous' > tag, define generalized accessor tagOrClassName which returns the > value of 'extends' if it exists, otherwise return the tag name > > > Compiler.java: remove some of the class inlining code that has not worked in > forever > > DebugCompiler.java: remove class inlining code > > > ClassModel.java: make the ClassModel constructor accept a <anonymous> tag > that has mixins, and build the > interstitial classes just like for a <class> that has mixins. > > > > > > > Tests: > > test/lztest/lztest-mixins.lzx added to the "ant lztest" suite > > test/smoke/mixin-simple.lzx > > testcase from bug (uncomment the commented out region) > > > Files: > M test/lztest/rhino.txt > A test/lztest/lztest-mixins.lzx > A test/smoke/mixin-simple.lzx > M WEB-INF/lps/schema/lfc-undeclared.lzx > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ToplevelCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewSchema.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/Compiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/DebugCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java > > Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20091120-hqm-i.tar > > _______________________________________________ > Laszlo-reviews mailing list > [email protected] > http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews _______________________________________________ Laszlo-reviews mailing list [email protected] http://www.openlaszlo.org/mailman/listinfo/laszlo-reviews
