Thanks Tucker, I made that fix and committed.
- Don On May 4, 2011, at 7:51 AM, P T Withington wrote: > This looks great. > > My only comment is that rather than using $tag$ as the prefix for the non-LZX > classes, perhaps use something more descriptive, say $lfc$ (indicating that > it is an internal LFC class. > > > On 2011-04-22, at 11:50, Donald Anderson wrote: > >> Tucker, I'm resending this review after 'svn up'. Do you have time to >> review this? >> - Don >> >> Change dda-20110407-qa4 by [email protected] on 2011-04-07 09:46:30 EDT >> in /Users/dda/laszlo/src/svn/openlaszlo/trunk-c >> for http://svn.openlaszlo.org/openlaszlo/trunk >> >> Summary: Emit reference classes for all LFC classes that do not have tags >> >> New Features: >> >> Bugs Fixed: LPP-9873 : with(this) emitted for in class extending LzEventable >> >> Technical Reviewer: ptw (pending) >> QA Reviewer: (pending) >> Doc Reviewer: (pending) >> >> Documentation: >> >> Release Notes: >> >> Overview: >> In previous commits, the issue of having reference classes for LFC >> classes that were not associated with tags was deferred. This >> commit fixes that deficiency. The primary mechanism is for the >> js2doc 'schemabuilder' to create dummy lzx tags for any public LFC >> javascript class that does not have an lzx tag. For example >> LzEventable is given tag "$tag$LzEventable". A tag name starting >> with '$' is illegal to use in XML, so it amounts to a private >> name, used internally only to identify inheritance. This approach >> firmly establishes a 1-1 relationship between tags and Javascript >> classes, and allows these classes to be emitted as reference classes >> while keeping the tag compiler clean. >> >> The end result is that: >> - methods of classes written in script that extend (previously) >> non-tagged classes, such as LzEventable, will now be >> considered for 'with(this) removal' >> - The LFC class heirarchy is now completely known, except for >> method arg types, and can be used for other purposes: helping >> to identify globals, helping to evaluate 'is' expressions, etc. >> >> Details: >> test/lztest/lztest-class-impl.lzx >> - added test case that shows the error in LPP-9873. >> If a with(this) is generated for that case, >> the 'pragma errorWithThis' results in a compile error >> (that behavior was tested too). >> - Added 'pragma errorWithThis' to several existing tests to >> ensure that with(this) is not emitted as a regression >> in the future. >> >> test/smoke/compiler.lzl >> - disabled some test functions that now fail given the 'complete >> knowledge'. >> For these, with(this) removal is now errantly done, even though a new >> transformation is needed, see >> http://jira.openlaszlo.org/jira/browse/LPP-9822 . >> - added tests for Aliasing using a non-LFC class to parallel tests >> that use >> an LFC class. These remain partially disabled due to LPP-9822 . >> >> WEB-INF/lps/schema/lfc-undeclared.lzx >> - Removed 'setAttribute' and 'ondestroy' from the list of >> added methods/events for <node>. These are from LzEventable >> (which node inherits from), and had been inserted previously >> to cover for this class missing from the reference class hierarchy. >> >> WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java >> - For classes that do not have an lzxname, the class name >> prefixed by "$tag$" is used. >> - Guaranteeing that all emitted classes have an lzxname cleans >> up some code. >> >> WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassCompiler.java >> - Removed special case code for a class with null tagName and non-null >> jsname. >> - Changed addClassModel() signature to pass through the jsname, if it >> exists, >> into the ClassModel. >> >> WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewCompiler.java >> WEB-INF/lps/server/src/org/openlaszlo/compiler/TypeCompiler.java >> WEB-INF/lps/server/src/org/openlaszlo/compiler/ViewSchema.java >> WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java >> - Changed addClassModel() signature to pass through the jsname, if it >> exists, >> into the ClassModel. >> >> WEB-INF/lps/server/src/org/openlaszlo/compiler/ClassModel.java >> - className is now a constructor arg, if it exists, it will be the JS >> name. >> >> >> Tests: >> - smoke, lztest, runlzunit >> (lztest incorporates a test that shows the original problem) >> >> - Examined .js produced in test/lztest, all but 20 with(this) occurrences >> have gone away (there were about 100 before, most attributable to >> inheritance from LzEventable). Of the remaining 20, 1 of these is >> an intentional test case that uses with(somevariable), the others >> result from an explicit with(this) used in: >> lps/components/lztest/lztestmanager.lzx >> that is included in all the test cases. >> >> Files: >> M test/lztest/lztest-class-impl.lzx >> M test/smoke/compiler.lzl >> M WEB-INF/lps/schema/lfc-undeclared.lzx >> M WEB-INF/lps/server/src/org/openlaszlo/js2doc/SchemaBuilder.java >> 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/TypeCompiler.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/ClassModel.java >> M build-tools/runlzunit.sh >> >> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/dda-20110407-qa4.tar >> >> >> >> -- >> >> Don Anderson >> Java/C/C++, Berkeley DB, systems consultant >> >> voice: 617-306-2057 >> email: [email protected] >> www: http://www.ddanderson.com >> blog: http://libdb.wordpress.com >> >> >> >> >> > -- Don Anderson Java/C/C++, Berkeley DB, systems consultant voice: 617-306-2057 email: [email protected] www: http://www.ddanderson.com blog: http://libdb.wordpress.com
