André: Thanks for your careful review. Very appreciated.
On 2008-03-13, at 16:13 EDT, André Bargull wrote:
Why renaming LzSelectionManager and LzDataSelectionManager to
LzSelectionmanager and LzDataselectionmanager? I thought all LFC
classes (should) have camel-case, like LzLazyReplicationManager,
LzResizeReplicationManager, LzModeManager, LzAnimatorGroup, etc.
This was an early attempt at a uniform mapping of tagname <->
classname, and needs to be backed out.
Hmm, the "$lzc$set_"-prefix doesn't really convince me, that looks
like it should be actually handled by the compiler or wherever..
What do other languages do to declare setters?
Yes, this is a half-step, that needs to be cleaned up eventually. JS2
uses this syntax:
function set foo (v) { ... }
Which we should eventually adopt (see LPP-5587). One issue we have is
that our present system allows you to use the attribute as the 'state'
of the setter, which would lead to infinite recursion if we were to
use JS2 setters (since setting the state in the setter would call the
setter). We'd need to clean that up to take advantage of the runtimes
that implement setters.
How smart is the compiler, does it recognize setters, if not, using
setAttribute would be really expensive:
setAttribute("foo", 1) -> calls $lzc$set_foo(1) -> and this calls
finally setFoo(1)
setAttribute will be in-lined, eliminating one step. The other two
steps are already there, because a setter is already compiled as a
closure. I am just making the setter be a method now. In any new
code, we should just _not_ write the setter as a separate method.
`setFoo` is a hold-over from old-style of writing LZX, which we
(sadly) have to maintain for backwards-compatibility (apparently),
even though our documentation has said for any number of releases that
you should be calling setAttribute and not setFoo. (And, if we ever
got to use real JS2 setter methods, setAttribute could be compiled
away completely.
Why deprecating these methods in LzAnimator(Group)?
LzAnimatorGroup: setTarget, setDuration, setStart
LzAnimator: setMotion, setTo
Because they were already private, and, as I explained above, they are
old-style. You want to access these through setAttribute. The setter
method should be primary, and we should provide trampolines for the
old API's for back-compatibility.
") ?
- var tooltip = this.getAttribute("tooltip_obj");
+ var tooltip = this.tooltipbj");
Fixed
"hisetAttribute"?
- var keyID =
component.getAttribute(this.getAttribute("component_identifier"));
- var msg = this.getAttribute("messageMap")[keyID];
+ var keyID = component.hisetAttribute("component_identifier"));
+ var msg = this.messageMap[keyID];
Fixed
missing $debug
var p = this.immediateparent;
+ var anc = [];
while (p != canvas) {
- if (p instanceof basecomponent) {
+ anc.push(p)
+ if (p instanceof lz.basecomponent) {
this._parentcomponent = p;
break;
}
p = p.immediateparent;
}
+ if (! this._parentcomponent) {
+ Debug.debug("No parent for %s in %s", this, anc);
+ }
+
Fixed (by removing altogether).
BTW, poor reviewers, GLHF :o)
Thanks again for your review!
