[Laszlo-reviews] For Review: Change 20100604-ptw-7 Summary: Remove earlySetters kludges

André Bargull andre.bargull at udo.edu
Tue Jun 8 15:38:34 PDT 2010


> +      } else if (key == 'name') {
> +        // `name` needs to be set ASAP
> +        this.$lzc$set_name(args.name);

Performing this check costs an extra string comparison for each loop 
iteration. If the name needs to be set as soon as possible, it should be 
done before entering the loop.



> +      var p = this.parent;
> +      if ($classrootdepth) {
> +        while (--$classrootdepth > 0) { p = p.parent }
> +      }
> +      this.classroot = p;

Change introduces different semantics here, classroot is set to parent 
even if args.$classrootdepth is 0 (or any other 'falsy' value).



> +    if (('$datapath' in args) && (args.$datapath !== ignore)) {
> +      var $datapath = args.$datapath;
> +      args.$datapath = ignore;

Why do you set args.$datapath to ignore?



> -  /** @access private */
> -  var handlerMethodNames;

How did handlerMethodNames actually work? It's filled in 
lz.State#set_$delegates() which is called in lz.Node#applyArgs() which 
in turn gets called at the end of lz.State#applyArgs(). But 
lz.State#applyArgs() is already using handlerMethodNames right at the 
beginning, at that time it should still be empty, shouldn't it?



> +  /** @access private */
> +  public function LzState ( parent:* , attrs:* , children:* = null, instcall:*  = null) {
> +    // Must init before super because used by __LZapplyArgs override
> +    // which will be called from super!
> +    this.heldArgs = {};
> +    this.appliedChildren = [];
> +    super(parent,attrs,children,instcall);
> +  }

I don't understand the motivation for this change. And also: 
http://openlaszlo.org/pipermail/laszlo-dev/2008-July/016411.html




> +  /*
> +   * Special flags to keep these psuedo-args from being processed
> +   */

"pseudo" ;-)



> +  /*
> +   * Special flags to keep these psuedo-args from being processed
> +   */

[...] from being processed "by __LZapplyArgs()"



Otherwise looks good, but I haven't done any tests yet.



On 6/8/2010 8:42 PM, P T Withington wrote:
> Change 20100604-ptw-7 by ptw at padme.home on 2010-06-04 08:47:02 EDT
>      in /Users/ptw/OpenLaszlo/trunk
>      for http://svn.openlaszlo.org/openlaszlo/trunk
>
> Summary: Remove earlySetters kludges
>
> Bugs Fixed:  LPP-7354 Presentation Types (partial substrate cleanup)
>
> Technical Reviewer: andre.bargull at udo.edu (pending)
> QA Reviewer: max at openlaszlo.org (pending)
>
> Overview:
>
>      `earlySetters` were an attempt by Adam to make a general mechanism,
>      but each of the actual usages is a special case.  We don't support
>      this generality outside of the kernel (it is handled instead by
>      overriding the construct method), so I'm removing it.  It's just a
>      lot of extra fragile mechanism that makes the init args more
>      difficult to process and interferes with attribute annotation.
>
> Details:
>
>      LzNode, LaszloView, LaszloCanvas:  remove earlySetters lists.
>
>      LzNode: Remove unused `doneClassRoot` flag, unused `defaultSet`
>      method.  Handle the previous earlySetters explicitly in
>      __LZapplyArgs.  For those that are only used to communicate
>      between the compiler and the runtime ($delegates, $classrootdepth,
>      $datapath), hoist the former setters inline (but keep the `-1`
>      sentinel that indicates they are handled specially in
>      __LZapplyArgs).
>
>      LaszloView: Handle `clickregion`, formerly an earlySetter, in
>      construct.
>
>      LzState: Clean up class organization to match standard order.
>      Remove unneeded initial value for __LZpool.  Make the constructor
>      do something useful.  Hide unnecessary documentation of implicit
>      event.  Remove 4.1-era deprecated API's.  Handle sorting
>      `$delegates` between self and parent explicitly in __LZapplyArgs
>
> Tests:
>      smokecheck, selected demos
>
> Files:
> M       WEB-INF/lps/lfc/core/LzNode.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
>
> Changeset: http://svn.openlaszlo.org/openlaszlo/patches/20100604-ptw-7.tar
>


More information about the Laszlo-reviews mailing list