[webkit-reviews] review granted: [Bug 135763] Web Inspector: Introduce an inspector Abstract Syntax Tree. : [Attachment 236674] patch

[bugzilla-daemon at webkit.org]

Joseph Pecoraro <joepeck at webkit.org> has granted Saam Barati
<sbarati at apple.com>'s request for review:
Bug 135763: Web Inspector: Introduce an inspector Abstract Syntax Tree.
https://bugs.webkit.org/show_bug.cgi?id=135763

Attachment 236674: patch
https://bugs.webkit.org/attachment.cgi?id=236674&action=review

------- Additional Comments from Joseph Pecoraro <joepeck at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=236674&action=review


Looks good to me. A few style ignorable style nits.

> Source/WebInspectorUI/UserInterface/Models/Script.js:135
> +	   if (this._scriptSyntaxTree) {
> +	       setTimeout(function() { callback(this._scriptSyntaxTree);
}.bind(this), 0);

Now that I read this, I think this might be more efficient, avoiding the bind.

    setTimeout(function(tree) { callback(tree); }, 0, this._scriptSyntaxTree);

> Source/WebInspectorUI/UserInterface/Models/Script.js:140
> +	   var makeSyntaxTreeAndCallCallback = function(content)
> +	   {

Style: I would prefer this style for the closure:

    function makeSyntaxTreeAndCallCallback(content) {
	...
    }.bind(this)

> Source/WebInspectorUI/UserInterface/Models/Script.js:207
> +    _makeSyntaxTree: function(sourceText) {

Style: brace on new line.