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

[P T Withington]

Agreed.

So, approved?

On 2008-02-14, at 16:18 EST, André Bargull wrote:

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 [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