RESOLVED FIXED 62367
Web Inspector: Make it obvious where command line functions come from
https://bugs.webkit.org/show_bug.cgi?id=62367
Summary Web Inspector: Make it obvious where command line functions come from
Chris Broadfoot
Reported 2011-06-09 04:35:00 PDT
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.
Attachments
Patch (5.42 KB, patch)
2013-02-27 09:13 PST, Mike West
no flags
Patch (5.90 KB, patch)
2013-02-27 09:35 PST, Mike West
no flags
Patch (7.84 KB, patch)
2013-02-28 01:26 PST, Mike West
no flags
Patrick Mueller
Comment 1 2011-06-09 06:30:19 PDT
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
Comment 2 2011-06-09 06:33:21 PDT
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
Comment 3 2013-02-27 09:13:48 PST
Mike West
Comment 4 2013-02-27 09:35:58 PST
Mike West
Comment 5 2013-02-27 09:36:59 PST
(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 6 2013-02-27 10:21:34 PST
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 7 2013-02-27 10:27:43 PST
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 8 2013-02-27 13:17:26 PST
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
Comment 9 2013-02-28 01:26:18 PST
Mike West
Comment 10 2013-02-28 01:28:03 PST
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 11 2013-02-28 03:37:43 PST
Comment on attachment 190675 [details] Patch Clearing flags on attachment: 190675 Committed r144288: <http://trac.webkit.org/changeset/144288>
WebKit Review Bot
Comment 12 2013-02-28 03:37:48 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.