Web Inspector: Make it obvious where command line functions come from

[Chris Broadfoot]

At the moment, if you log $ to the console, you get the quite cryptic response:

bound: function ()
{
return document.getElementById.apply(document, arguments)
}

I spent a while wondering where it came from! It should be more obvious that this function is scoped to the console and not actually in the page.

Adding a comment to the toString (http://trac.webkit.org/browser/trunk/Source/WebCore/inspector/InjectedScriptSource.js) like:

// Command Line API support

would be great.

[Patrick Mueller]

Sounds like you're suggesting that we add a comment to the source where the function is defined.  Perhaps it would be better, for these functions, if we actually gave them a toString() method which returned something like: "[web inspector console function]".  

Or we could be more elaborate and have them print some help about themselves.

[Chris Broadfoot]

That sounds good, but...

I kind of like the toString() showing the actual implementation, though. I (and I assume other developers) can see immediately that $ maps to getElementById... and $$ maps to querySelectorAll.

I guess native functions aren't any better - so maybe "[web inspector console function]" is good enough!

[Mike West]

Created attachment 190540 [details]
Patch

[Mike West]

Created attachment 190543 [details]
Patch

[Mike West]

(In reply to comment #4)
> Created an attachment (id=190543) [details]
> Patch

This patch models the toString method after that of native code: "function $() { [WebInspector Command-Line Method] }".

[Build Bot]

Comment on attachment 190543 [details]
Patch

Attachment 190543 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-commit-queue.appspot.com/results/16758150

New failing tests:
inspector/console/command-line-api.html

[WebKit Review Bot]

Comment on attachment 190543 [details]
Patch

Attachment 190543 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://webkit-commit-queue.appspot.com/results/16826100

New failing tests:
inspector/console/command-line-api.html

[Pavel Feldman]

Comment on attachment 190543 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=190543&action=review

> Source/WebCore/inspector/InjectedScriptSource.js:1123
> +    function customToStringMethod(arg) {

Please follow the style guide for function declaration.

> Source/WebCore/inspector/InjectedScriptSource.js:1124
> +        return function () { return "function " + arg + "() { [WebInspector Command-Line Method] }"; };

Please use "Command Line API" string. It is port agnostic and can be searched for.

[Mike West]

Created attachment 190675 [details]
Patch

[Mike West]

Thanks for taking a look, Pavel!

(In reply to comment #8)
> (From update of attachment 190543 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=190543&action=review
> 
> > Source/WebCore/inspector/InjectedScriptSource.js:1123
> > +    function customToStringMethod(arg) {
> 
> Please follow the style guide for function declaration.

Gah. I'll learn...

> 
> > Source/WebCore/inspector/InjectedScriptSource.js:1124
> > +        return function () { return "function " + arg + "() { [WebInspector Command-Line Method] }"; };
> 
> Please use "Command Line API" string. It is port agnostic and can be searched for.

Done.

[WebKit Review Bot]

Comment on attachment 190675 [details]
Patch

Clearing flags on attachment: 190675

Committed r144288: <http://trac.webkit.org/changeset/144288>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.