[webkit-reviews] review requested: [Bug 26350] Make WebInspector's console evaluation/completion asynchronous. : [Attachment 31254] patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Sun Jun 14 00:33:32 PDT 2009
Pavel Feldman <pfeldman at chromium.org> has asked for review:
Bug 26350: Make WebInspector's console evaluation/completion asynchronous.
https://bugs.webkit.org/show_bug.cgi?id=26350
Attachment 31254: patch
https://bugs.webkit.org/attachment.cgi?id=31254&action=review
------- Additional Comments from Pavel Feldman <pfeldman at chromium.org>
(In reply to comment #4)
> (From update of attachment 31232 [review])
>
> This can just be written as:
>
> setTimeout(completionsReadyCallback, 0, results);
>
Done.
>
> The open brace should be on the next line for these.
>
Done.
>
> I think it would be cleaner to define a nested function then call setTimeout
on
> it, not using an anonymous function inline.
>
Done.
>
> Same here.
>
Done.
> > -
> > +
>
> Extra space added here.
>
Not sure which one you mean. How do I find exact line number?
>
> Does this work when case dosen't match? The completions are found
> case-insenitvly and indexOf is case-sensitive.
>
In fact, I think that completions are also case-sensitive. At least that is
what I've learned from playing with the tool.
> 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.
>
Done. Regardless of case sensitivity I could do it this way.
More information about the webkit-reviews
mailing list