[Webkit-unassigned] [Bug 26350] Make WebInspector's console evaluation/completion asynchronous.

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26350


timothy at hatcher.name changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31232|review?                     |review-
               Flag|                            |




------- Comment #4 from timothy at hatcher.name  2009-06-13 14:01 PDT -------
(From update of attachment 31232)
> +        setTimeout(function() {
> +            completionsReadyCallback(results);
> +        }, 0);

This can just be written as:

setTimeout(completionsReadyCallback, 0, results);

> +    _ensureCommandLineAPIInstalled: function(inspectedWindow) {
> +    doEvalInWindow: function(expression, callback) {
> +        function printResult(result, exception) {
> +        function updatingCallbackWrapper(result) {

The open brace should be on the next line for these.

> +        setTimeout(function() {
> +            var inspectedWindow = InspectorController.inspectedWindow();
> +            self._ensureCommandLineAPIInstalled(inspectedWindow);
> +            try {
> +                callback(inspectedWindow.eval(expression));
> +            } catch (e) {
> +                callback(e, true);
> +            }
> +        }, 0);

I think it would be cleaner to define a nested function then call setTimeout on
it, not using an anonymous function inline.

> +        setTimeout(function() {
> +            try {
> +                callback(callFrame.evaluate(code));
> +            } catch (e) {
> +                callback(e, true);
> +            }
> +        }, 0);

Same here.

> -
> + 

Extra space added here.

> +        var currentText = fullWordRange.toString();
> +
> +        var matchingCompletions = []
> +        for (var i = 0; i < completions.length; ++i) {
> +            if (completions[i].indexOf(currentText) == 0)
> +                matchingCompletions.push(completions[i]);
> +        }  
> +        completions = matchingCompletions;
> +
> +        if (!completions || !completions.length)
> +            return;

Does this work when case dosen't match? The completions are found
case-insenitvly and indexOf is case-sensitive.

I think a better way to detect instead of looking at all the completion
prefixes, would be to get the word range that was present when completions was
made. If the word range matches the current fullWordRange, then it is the right
one. Otherwise return early when there isn't a match and don't call
this.clearAutoComplete in that case.

How does that sound to you?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.