Summary: | Web Inspector: Make it obvious where command line functions come from | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Chris Broadfoot <cb> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Mike West <mkwst> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | apavlov, buildbot, bweinstein, dglazkov, graouts, joepeck, keishi, loislo, mkwst, paulirish, pfeldman, pmuellr, rik, rniwa, timothy, webkit.review.bot, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Chris Broadfoot
2011-06-09 04:35:00 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. 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! Created attachment 190540 [details]
Patch
Created attachment 190543 [details]
Patch
(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] }". 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 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 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. Created attachment 190675 [details]
Patch
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. Comment on attachment 190675 [details] Patch Clearing flags on attachment: 190675 Committed r144288: <http://trac.webkit.org/changeset/144288> All reviewed patches have been landed. Closing bug. |