Closed
Bug 924466
Opened 11 years ago
Closed 11 years ago
integrate acorn with the devtools
Categories
(DevTools :: Debugger, defect, P3)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 28
People
(Reporter: bbenvie, Assigned: bbenvie)
References
Details
(Whiteboard: [sec-review][qa-])
Attachments
(1 file, 4 obsolete files)
(deleted),
patch
|
rcampbell
:
review+
|
Details | Diff | Splinter Review |
We'd like to add a third party JavaScript parser to the tree because Reflect.parse isn't adequate for our needs [1]. The two options we have that produce compatible AST are acorn [2] and esprima [3]. acorn pros: * faster * succinct code (~40% the code to do the same thing) * better error-recovering parser esprima pros: * better tokenizer exposed * has support for most ES6 features (in its harmony branch) [1] https://groups.google.com/forum/#!topic/mozilla.dev.developer-tools/cWANPm0FWVM [2] https://github.com/marijnh/acorn [3] https://github.com/ariya/esprima
Assignee | ||
Comment 1•11 years ago
|
||
The decision here also would have an impact on which tool we use for code completion. Tern [1] depends on acorn, while aulx [2] depends on esprima. These tools rely on the lenient parsing modes and tokenizers that each of the parsers provide, which don't have compatible APIs.
Assignee | ||
Comment 2•11 years ago
|
||
[1] https://github.com/marijnh/tern [2] https://github.com/espadrine/aulx
Updated•11 years ago
|
Priority: -- → P3
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → bbenvie
Assignee | ||
Comment 3•11 years ago
|
||
After discussing it with a number of people, the consensus is to use acorn and to address the things it is missing via either contributing changes back to acorn or to ask acorn's owner (Marijn Haverbeke) about getting them implemented.
Comment 4•11 years ago
|
||
Be vary that this means that we will have to put some (maybe a lot of ? ) extra effort in: - Making the Acorn's lexar better (as compared to esprima) - Having the support of ES6 up to the level so that ACorn is actually useful in Chrome tools like browser debugger, browser level scratchpad, etc. [This is not a discussion, I am okay with any option. Just wanted put it out here amidst the fact that we are already getting bitten by the lack of ES6 support in other external tools like CM, escodegen etc.]
Assignee | ||
Comment 5•11 years ago
|
||
Most of the work is done here. I'd probably like to add some more tests though.
Assignee | ||
Comment 6•11 years ago
|
||
Bug 932416 is the legal review submission.
Assignee | ||
Updated•11 years ago
|
Whiteboard: [sec-review]
Assignee | ||
Comment 7•11 years ago
|
||
Fixes test_same_ast.js, adds test_parse_dammit.js.
Attachment #824158 -
Attachment is obsolete: true
Assignee | ||
Updated•11 years ago
|
Summary: integrate a third party JavaScript parser with the devtools → integrate acorn with the devtools
Assignee | ||
Comment 8•11 years ago
|
||
Adds acorn to license.html.
Attachment #824255 -
Attachment is obsolete: true
Assignee | ||
Comment 9•11 years ago
|
||
Clean up the tests a bit. https://tbpl.mozilla.org/?tree=Try&rev=1c3c634bf814
Attachment #824265 -
Attachment is obsolete: true
Assignee | ||
Comment 10•11 years ago
|
||
Comment on attachment 824287 [details] [diff] [review] acorn.patch robcee for the general review dcamp for loader changes gps for adding a new folder to the build system
Attachment #824287 -
Flags: review?(gps)
Attachment #824287 -
Flags: review?(dcamp)
Assignee | ||
Updated•11 years ago
|
Attachment #824287 -
Flags: review?(rcampbell)
Comment 11•11 years ago
|
||
Comment on attachment 824287 [details] [diff] [review] acorn.patch Review of attachment 824287 [details] [diff] [review]: ----------------------------------------------------------------- Covers just the build bits.
Attachment #824287 -
Flags: review?(gps) → review+
Comment 12•11 years ago
|
||
Comment on attachment 824287 [details] [diff] [review] acorn.patch Review of attachment 824287 [details] [diff] [review]: ----------------------------------------------------------------- I hate to be that guy, but could we name the test test_lenient_parser.js?
Assignee | ||
Comment 13•11 years ago
|
||
(In reply to Dave Camp (:dcamp) from comment #12) > I hate to be that guy, but could we name the test test_lenient_parser.js? Hah, of course.
Comment 14•11 years ago
|
||
Comment on attachment 824287 [details] [diff] [review] acorn.patch Review of attachment 824287 [details] [diff] [review]: ----------------------------------------------------------------- Looks ok to me, assuming that file is renamed.
Attachment #824287 -
Flags: review?(dcamp) → review+
Assignee | ||
Comment 15•11 years ago
|
||
Rebase and a few changes: * Rename test_parse_dammit.js to test_lenient_parser.js * Remove package.json.js since it's not actually needed
Attachment #824287 -
Attachment is obsolete: true
Attachment #824287 -
Flags: review?(rcampbell)
Attachment #825584 -
Flags: review?(rcampbell)
Comment 16•11 years ago
|
||
Comment on attachment 825584 [details] [diff] [review] acorn.patch Review of attachment 825584 [details] [diff] [review]: ----------------------------------------------------------------- looks ok to me too.
Attachment #825584 -
Flags: review?(rcampbell) → review+
Comment 17•11 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/7e81c446b6ca
Whiteboard: [sec-review] → [sec-review][fixed-in-fx-team]
Comment 18•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7e81c446b6ca
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Whiteboard: [sec-review][fixed-in-fx-team] → [sec-review]
Target Milestone: --- → Firefox 28
Updated•11 years ago
|
Whiteboard: [sec-review] → [sec-review][qa-]
Comment 19•10 years ago
|
||
Sorry for picking up this old bug, but do you have a tracker for porting ES6 features to acorn?
Comment 20•10 years ago
|
||
I don't think so. Bug 938203 also depends on acorn supporting ES6.
Comment 21•10 years ago
|
||
Acorn ES6 support is coming in https://github.com/marijnh/acorn/pull/110.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•