Summary: | Web Inspector: modify generate_protocol_externs.py to generate JSON typedef's for @constructors | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andrey Adaikin <aandrey> | ||||||||
Component: | Web Inspector (Deprecated) | Assignee: | Andrey Adaikin <aandrey> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | apavlov, buildbot, dglazkov, keishi, loislo, pfeldman, pmuellr, rniwa, vsevik, web-inspector-bugs, webkit.review.bot, yurys | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Andrey Adaikin
2013-01-18 08:43:54 PST
Created attachment 183506 [details]
Patch
Created attachment 183515 [details]
Generated protocol externs diff
Comment on attachment 183506 [details] Patch Attachment 183506 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/15948481 New failing tests: inspector/console/command-line-api.html Comment on attachment 183506 [details] Patch Attachment 183506 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15957237 New failing tests: inspector/console/command-line-api.html Comment on attachment 183506 [details] Patch Attachment 183506 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/15949458 New failing tests: inspector/console/command-line-api.html Comment on attachment 183506 [details]
Patch
What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated.
(In reply to comment #6) > (From update of attachment 183506 [details]) > What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated. hmmm... plz see my first comment :) also plz check out the generated protocol externs diff. These are two different types. The CanvasAgent.TraceLog defines a type of CanvasAgent.TraceLog class with some methods defined in prototype and etc. The CanvasAgent.TraceLogJSON is of the Object type with a set of properties. (In reply to comment #7) > (In reply to comment #6) > > (From update of attachment 183506 [details] [details]) > > What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated. > > hmmm... plz see my first comment :) also plz check out the generated protocol externs diff. > > These are two different types. The CanvasAgent.TraceLog defines a type of CanvasAgent.TraceLog class with some methods defined in prototype and etc. The CanvasAgent.TraceLogJSON is of the Object type with a set of properties. But I think they are equivalent from the compiler standpoint. At least the ones you specify in comment #1 are equivalent. (In reply to comment #8) > (In reply to comment #7) > > (In reply to comment #6) > > > (From update of attachment 183506 [details] [details] [details]) > > > What is CanvasAgent.TraceLogJSON? CanvasAgent.TraceLog is already generated. > > > > hmmm... plz see my first comment :) also plz check out the generated protocol externs diff. > > > > These are two different types. The CanvasAgent.TraceLog defines a type of CanvasAgent.TraceLog class with some methods defined in prototype and etc. The CanvasAgent.TraceLogJSON is of the Object type with a set of properties. > > But I think they are equivalent from the compiler standpoint. At least the ones you specify in comment #1 are equivalent. Those are different. Example: /** @type {CanvasAgent.TraceLog} */ var a; /** @type {CanvasAgent.TraceLogJSON} */ var b; a = {id:1, calls:[], ...}; // compiler warns it cannot be casted to CanvasAgent.TraceLog b = {id:2, calls:[], ...}; // compiler will do the right thing b = {calls:[], ...}; // compiler will warn that the 'id' property is missing, cause it's not optional Even the following is not permitted by the recent version of compiler: a = /** @type {CanvasAgent.TraceLog} */ ({id:1, calls:[], ...}); not to mention that we would loose all compiler type checking work. My point is that CanvasAgent.TraceLog is a type we define for protocol type TraceLog that is supposed to describe the JSON payload. And with that regard, TraceLog is no different from any other type we use. If you are saying compiler could check more if we switched to another notation, please switch entire protocol there. I.e. start generating @typedefs for all payloads and use existing names for the types. (In reply to comment #10) > My point is that CanvasAgent.TraceLog is a type we define for protocol type TraceLog that is supposed to describe the JSON payload. And with that regard, TraceLog is no different from any other type we use. If you are saying compiler could check more if we switched to another notation, please switch entire protocol there. I.e. start generating @typedefs for all payloads and use existing names for the types. Oh, I see. Yes, we should use @typedef instead of @constructor, since those payloads are of type Object. I'll send a new patch soon. Created attachment 183759 [details]
Patch
Comment on attachment 183759 [details] Patch Attachment 183759 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/16038016 New failing tests: inspector/console/command-line-api.html Comment on attachment 183759 [details] Patch Attachment 183759 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://queues.webkit.org/results/16038034 New failing tests: inspector/console/command-line-api.html Comment on attachment 183759 [details] Patch Attachment 183759 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/16043028 New failing tests: inspector/console/command-line-api.html Committed r140329: <http://trac.webkit.org/changeset/140329> |