[Laszlo-dev] For Review: Change 20080208-ptw-T Summary: Unify argument initialization

[André Bargull]

Just a tiny change in LzState#__LZapplyArgs(..):
you may want to change it to "var val = .." in the rename-loop, surely 
it isn't needed because of the ecma scoping-rules, but most times we do 
it that way.


On 2/14/2008 9:55 PM, P T Withington wrote:
> 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 ptw at dueling-banjos.local 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: a.bargull at intensis.de (pending)
>>> QA Reviewer: henry.minsky at gmail.com (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
>>>
>
>