[Laszlo-dev] For Review: Change 20080827-ptw-k Summary:

[P T Withington]

On 2008-08-30, at 14:11EDT, André Bargull wrote:

> Great stuff, that will make live easier for everyone who needs to  
> touch the debugger.
> Right now I'm going through the files, until now just (mini-)source  
> changes:
>
> LzRuntime.lzs, encodeURIComponent: why not decodeURIComponent?,

Laziness, I didn't need it (yet).

> LzBootstrapDebugConsole#addText(..): string coercion looks odd,  
> belongs into the catch-clause?

Fixed.

> LzBootstrapDebugLogger#log(..): loadVariables() should be  
> loadVariablesNum(), right?

Again, legacy code that I chose not to change.

> LzBootstrapDebugService: _dbg_log_all_writes, why a global?

Legacy code.  I'll make it a debugger property, since only the  
debugger refers to it now.

> LzBootstrapDebugService#xmlEscape(): we really ought to consolidate  
> all our different xml-escape routines...

True.

> LzBootstrapDebugService, "var Debug": you've left out the "var"- 
> declaration for the generic and runtime-specific debuggers, why not  
> here? (as the first "var Debug" is declared in LzRuntime.lzs)

Actually, making the extra var declarations will silence the compiler  
warning, so I will add it everywhere.

> LzBootstrapDebugService, $reportNotFunction and  
> $reportUndefinedMethod: message concatenation looks odd

Legacy code.  It could be simplified.

> LzCompiler, $reportSourceWarning: "if (LzWarning)" is (now) always  
> true, so the else-clause can be removed

Fixed.

> LzBootstrapMessage#indexOf(..args) vs LzDebugMessage#indexOf(key):  
> not a compatible override

And unnecessary.  Fixed.

> LzBootstrapMessage#toString(), #toHTML(): timestamp is off by three  
> years ;-)

Wow you are thorough!  Fixed.

> LzBootstrapMessage: String.prototype.toHTML() uses  
> "Debug.xmlEscape", which is only available in debug-mode

Moved to LzBootstrapMessage

> LzSourceMessage: missing declaration of "message" in class

Fixed.

> LzSourceMessage: constructor typing wrong, "message" is either  
> String or LzMessage

Fixed.

> LzDebugService.internalPropertyPrefixes: use an Object instead of an  
> Array (it's used as an Object in LzDebugService#internalProperty())

Actually it's used as an Array and should be iterated over using an  
index, not for-in.  Fixed.

> debugger-component: update the comment at file-beginning and mark  
> the methods which implement the LzBootstrapDebugConsole-interface

> LzDHTMLDebugConsole declares "doEval" as overriden, but there is no  
> "doEval" in LzBootstrapDebugConsole, missing?

Indeed.  doEval is part of the console protocol:  the console is  
required to provide an evaluation mechanism, and to call back to  
Debug.displayResult with the value.  This allows evaluation on systems  
where there is not compiler -- the console creates a link to a server  
that can remotely compile.

I tried to clean this up a bit, but the console and debugger  
interfaces are evolving as needed.

> Does it make sense to replace these lines (occur several times in  
> the debugger-files):
>> var color = '#0000ff';
>> if (attrs && attrs.color) { color = attrs.color };
> with a shorter replacement (for LPP-6892): var color = (attrs &&  
> attrs.color) || '#0000ff';

Sure.

---

Review coming your way!