Summary: | Web Inspector: Add esprima to the WebInspector. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Saam Barati <saam> | ||||||||
Component: | Web Inspector | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | commit-queue, fpizlo, graouts, joepeck, timothy, webkit-bug-importer, ysuzuki | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | All | ||||||||||
OS: | All | ||||||||||
Attachments: |
|
Description
Saam Barati
2014-07-19 15:10:17 PDT
Created attachment 235176 [details]
patch
This patch includes Esprima into the WebInspector and attaches its exported function onto the WebInspector namespace object.
Comment on attachment 235176 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=235176&action=review > Source/WebInspectorUI/ChangeLog:11 > + * UserInterface/External/esprima.js: Added. Lets make a subfolder for this, like other External libraries. UserInterface/External/Esprima/esprima.js > Source/WebInspectorUI/UserInterface/External/esprima.js:1 > +/* We will want to minify this script in production builds. See how this script handles CodeMirror: Source/WebInspectorUI/Scripts/copy-user-interface-resources.pl This could be done in a follow-up. > Source/WebInspectorUI/UserInterface/External/esprima.js:34 > +WebInspector._esprima = {}; You should mention the edits you've made in a comment in this file. I suspect this, and its use below, are the only changes you made. Created attachment 235238 [details]
patch
This patch includes Esprima into the WebInspector and attaches its exported function onto the WebInspector namespace object.
(Will modify the minifying pearl script in another patch).
Comment on attachment 235238 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=235238&action=review > Source/WebInspectorUI/UserInterface/External/Esprima/esprima.js:35 > +// I delted the code that determines the context esprima is loaded in (node.js, the browser, etc.) and replaced it with WebInspector._esprima. We should not have "I" in a comment. Someone reading this will not know who that refers too, and any one individual does not necessary own a file. This is more of a "we" meaning the project / contributors as a whole. Something like this would be sufficient: // WebKit Modifications: // - replaced code that automatically determines Esprima execution context (node, browser, etc.) with WebInspector._esprima The idea here is to make it easier for the next person who updates Esprima by documenting the changes we've made. Created attachment 235252 [details]
patch
This patch includes Esprima into the WebInspector and attaches its exported function onto the WebInspector namespace object.
(Comment modifications taken into account).
Comment on attachment 235252 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=235252&action=review > Source/WebInspectorUI/ChangeLog:13 > + (.): Nit: prepare-ChangeLog left a turd. In the future you can remove these. Comment on attachment 235252 [details] patch Clearing flags on attachment: 235252 Committed r171334: <http://trac.webkit.org/changeset/171334> All reviewed patches have been landed. Closing bug. |