Thanks for the thorough review. I've updated the change with
responses to your comments, if you want to refetch it and check.
On 2008-02-14, at 14:09 EST, André Bargull wrote:
Current status, haven't applied your changes yet, will do it now.
LzNode#__LZapplyArgs(..):
- when you call the early setters, you defined a "setr" variable,
but it isn't used anywhere
Fixed.
- setters are now called in reversed order, I don't think we
guarantee anything about the setter-order, but it /may/ break any
fancy apps
You're right, this is a gratuitous change, I will reverse the loop.
(For is faster than pop, that's the only reason I changed it). We do
happen to know that 'for in' iterates in the opposite order in Flash
vs. Javascript, and since the setters list is built from a 'for in'
iteration, we can be pretty sure order is not important here. We know
it _is_ important for the early setters, which is why they are handled
with the numerical ordering.
LzNode#__LZstoreRefs():
- missing $debug
LzNode#__LZresolveRefs():
- missing $debug
LzNode#__LZresolveReferences():
- missing $debug
All fixed.
LzNode#applyConstraint(..):
- "constraintMethodName" shouldn't be a global variable
Well spotted!
LzNode#applyConstraintMethod(..):
- Maybe you should underline that the constraint-method must already
exist on the node. (Yes, I do know you've wrote "method" which says
implicitly it's already on the node, but I'm not quite sure, whether
everyone knows the exact differences between methods and functions.
And in the javadoc there is again "function".)
Fixed.
- don't you want to improve the delegate construction a bit, like:
var dp;
for (var i = 0; i < l; i += 2) {
dp = dependencies[i];
if (dp) {
var d = new LzDelegate(this, constraintMethodName, dp, "on" +
dependencies[i + 1]);
this.__LZdelegates.push(d);
}
}
Sure.
LzCanvas#__LZcallInit():
- "TODO: ask Max": canvas overwrites LzView#init(), most likely only
a legacy of pre-4 OL, I think you should be able to remove both
things, so the explicit call for initializing the sprite and the
overwrite of LzView#init()
I'm not going to mess with this in this change. If you feel strongly
about it, please file an improvement request.
LzState#remove():
- missing $debug
Fixed.
LzState#__LZapplyArgs(..):
- you iterate over the held-object and while you do that, you also
modify it. Questions: may performance suffer because of this? So,
does the runtime notice these changes, or does it discard them?
(Thinking of Java, where you can't simply change a Map while
iterating over it)
Good point. I _know_ this is safe in Flash, but it is poor practice
to modify something you are iterating over. I've re-written the loop
to accumulate the needed changes and then make the changes in a
separate loop.
You use a couple of times: "foo.bar instanceof LzInitExpr" and "bar"
can be undefined. Was it the instanceof- or the is-operator in AS3,
which makes problems when using it with undefined values? (for
example in LzReplicationManager#construct(..) you're doing this)
Well you should be allowed to have undefined as the lhs of
instanceof. If this is broken in swf9, we should fix it in the
compiler, not in our source. I prefixed these tests with an `in`
check anyways.
On 2/13/2008 9:43 PM, P T Withington wrote:
Change 20080208-ptw-T by [EMAIL PROTECTED] on 2008-02-08
08:18:39 EST
in /Users/ptw/OpenLaszlo/ringding
for http://svn.openlaszlo.org/openlaszlo/trunk
Summary: Unify argument initialization
Bugs Fixed:
LPP-1587 'ECMA4: Compile LZX declarations as JS
declarations' (partial)
LPP-631 'can't override a constraint with a literal with state
attributes'
Technical Reviewer: [EMAIL PROTECTED] (pending)
QA Reviewer: [EMAIL PROTECTED] (pending)
Details:
Initial values, initial expressions ($once), and constraints
($always), now are created as a unified list in the tag compiler
solving several issues with overriding/inheritance of attribute
values and constraints and enabling the solution to an issue with
states and constraints.
TextStprite: Update test for constraint on width or height.
LzNode, UserClass: Simplify attribute inheritance/override
computation,
maintain backward-compatible magic-merge of attribute values (for
now).
LzNode: Update documentation of `construct` to note that
constraints are now unified in `args`. __LZapplyArgs uses new
args organization to sort out initialization expressions and
constraints. __LZresolveReferences uses the new information to
run initialization expressions and install constraints.
__LZresolveRefs no longer needed. applyConstraint deprecated, but
backward-compatible. New interface: applyConstraintMethod.
Private releaseConstraint did not work, removed. New interface:
releaseConstraintMethod.
LzDefs: Turn LzDeclaredEvent into a singleton class. Add private
support classes LzInitExpr and LzConstraintExpr.
LaszloView: Update test for constraints. Fix setAlign and
setValign to work with constraint methods, and to correctly work
if alignment is changed (to release previous constraint).
LaszloCanvas: Update __LZcallInit to parallel LzNode more
closely.
LzState: Update capturing of held args and constraints. Use this
new organization to release constraints the state will override
(and re-apply them when the state is removed). Remmove unused
__LZstoreRefs.
LzReplicationManager: Fix xpath constraint to work with
constraint
methods.
Function.java: Make Function#name public.
NodeModel.java: Coalesce $once, $path, $always into single
arglist, distinguishing 'binders' by internal type, either
LzInitExpr or LzConstraintExpr.
Tests:
smokecheck, amazon, most-components
Files:
M WEB-INF/lps/lfc/kernel/swf/LzInputTextSprite.as
M WEB-INF/lps/lfc/debugger/LzMessage.lzs
M WEB-INF/lps/lfc/core/LzNode.lzs
M WEB-INF/lps/lfc/core/UserClass.lzs
M WEB-INF/lps/lfc/core/LzDefs.lzs
M WEB-INF/lps/lfc/views/LaszloView.lzs
M WEB-INF/lps/lfc/views/LaszloCanvas.lzs
M WEB-INF/lps/lfc/helpers/LzState.lzs
M WEB-INF/lps/lfc/data/LzReplicationManager.lzs
M WEB-INF/lps/server/src/org/openlaszlo/sc/Function.java
M WEB-INF/lps/server/src/org/openlaszlo/compiler/NodeModel.java
Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20080208-ptw-T.tar
