Not approved yet. You need to add -ea to your JAVA_OPTS. You are apparently not enforcing assertions, because there are several that should have failed.
You need to add a test case for an instance of something other than view plus more than one mixin to really test this. --- There is something wrong with your changes at ClassModel lines 135-146: If the 'kind' is not class/interface/mixin, then the tagName should be promoted to the superTagName (hence the assertions that you moved out of the else clause). Basically, you should not have changed the else clause, you just needed to move the mixin parsing out of the then clause to below the entire if, so that it runs for either case. Then you would not have to comment out setting the tagName to `"anonymous " + superTagname`, which is helpful for debugging. As to your mystery about the CanvasCompiler iterator, see ToplevelCompiler updateSchema. The class mixins are created at updateSchema time, not at compile time. So, I think you will find there is a bug with this change, in that you cannot forward-reference a mixin from an instance. Thinking about it, this isn't really quite right. The transformation that is made when a class has mixins is: <class name="foo" extends="bar" with="bletch, crud">...</class> becomes: <class name="$crud$bar" extends="bar">...crud body...</class> <class name="$bletch$crud$bar" extends="crud$bar">...bletch body</class> <class name="foo" extends="$bletch$crud$bar">...foo body...</class> The transformation needed when an instance has mixins should be: <bar name="foo" with="bletch, crud">...</bar> becomes: <class name="$crud$bar" extends="bar">...crud body...</class> <class name="$bletch$crud$bar" extends="crud$bar">...bletch body</class> <$bletch$crud$bar name="foo">...foo body...</$bletch$crud$bar> With the current change, it looks like the instance declaration is turned into something like: <bar name="foo" extends=$bletch$crud$bar">...</bar> Which can't be right? You can't have bar extend itself. Maybe a better approach is to add an updateSchema method to ViewCompiler, patterned on the one in ClassCompiler, that will add the instance interstitials at schema time if the instance has mixins and re-write the DOM tag to be the most-specific interstitial (as illustrated above). Then the compilation phase will just do the right thing, and forward references should just work. On 2009-11-16, at 01:42, Henry Minsky wrote: > Change 20091116-hqm-B by [email protected] on 2009-11-16 01:22:06 EST > in /Users/hqm/openlaszlo/trunk > for http://svn.openlaszlo.org/openlaszlo/trunk > > Summary: allow mixin spec 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: > > Overview: > > LZX instances can now accept the "with" attribute to add mixins > > Details: > > schema/lfc-undeclared.lzx: > Move mixin declaration to <node> > > compiler/NodeModel.java: > Check if an instance has methods OR mixins, to decide whether to make > an anon class for it. > > compiler/CanvasCompiler.java: > Needed to make a copy of the child element list, so I don't get a > concurrent access exception from the > iterator. The issue is that the ClassModel mixin processor appends stuff to > the DOM, which would cause this issue. > I'm a little perturbed that I needed to do this, why wasn't this happening > before when regular class mixins were > getting compiled? > > compiler/ClassModel.java: > Factor out code to parse mixins, to make it easier to read. > > If we're modeling an instance rather than a class, use the tagname > as the superclass instead of the "extends" attribute val. > > > > Tests: > > smokecheck in swf10,dhtml,swf8 > > test case (use swf10 for most strict compiler test) > [note, I ought to make a lztest unit test for this, maybe to go into > smokecheck?] > > <canvas> > <simplelayout/> > <text>Each instance should have a 40x40 colored square. First two squares > will turn green when clicked on</text> > <class name="colored_square" bgcolor="#ccffcc" width="40" height="40"/> > > <!-- regular old instance which makes anon class --> > <view name="nomixin" width="40" height="40" bgcolor="#cccccc"> > <text>instance with no mixin</text> > <handler name="onclick"> > this.setAttribute('bgcolor', 0x00ff00); > </handler> > </view> > > > <!-- instance which uses mixins and makes anon class --> > <view name="mymixin" with="colored_square" > > <text>instance with mixin</text> > <handler name="onclick"> > this.setAttribute('bgcolor', 0x00ff00); > </handler> > </view> > > <!-- instance which has no methods so would not normally make an anon class, > except that it it uses a mixin--> > <view name="simplemixin" with="colored_square" bgcolor="#cc0000" > > <text>instance with mixin but no methods</text> > </view> > > > </canvas> > > > Files: > M WEB-INF/lps/schema/lfc-undeclared.lzx > M WEB-INF/lps/server/src/org/openlaszlo/compiler/CanvasCompiler.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java > M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java > > Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20091116-hqm-B.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
