escodegen will take a spidermonkey AST and pretty-ify it and make source maps too. If we can leverage this, we get 50% of the way to fixing bug 762761.

Lets you require("escodegen/escodegen") and generate code. Doesn't work with source maps because I can't figure out how to load SourceMap.jsm through the loader. In the loader's paths, if I add "source-map": "resource://gre/modules/devtools/SourceMap", then it tries to load "resource://gre/modules/devtools/SourceMap.js" // .js, not .jsm If I add "source-map": "resource://gre/modules/devtools/SourceMap.jsm", then it tries to load "resource://gre/modules/devtools/SourceMap.jsm.js" Halp! My goal is to integrate escodegen without modifying its source at all!

Should I just add SourceMap.js which just does: module.exports = Cu.import("resource://gre/modules/devtools/SourceMap.jsm", {}); ?

I think we can replace GCLI ` javascript beautifier (jsb)` command's backend with escodegen. This would be a appropriate way.

(In reply to Tetsuharu OHZEKI [UTC+9] from comment #3) > I think we can replace GCLI ` javascript beautifier (jsb)` command's backend > with escodegen. This would be a appropriate way. Yeah, also for the scratchpad pretty printing / deobfuscation bug (which I can't seem to find at the moment) will be able to use this.

You have a few options to load the jsm from the loader: * Rename it to .js * Talk to irakli about how to best add support for .jsm to loader.js * You could add a getter to Loader.jsm's "modules" thing that imports the jsm

Comment on attachment 796083 [details] [diff] [review] integrate-escodegen.patch Review of attachment 796083 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/devtools/Loader.jsm @@ +61,5 @@ > > + "escodegen/escodegen": "resource://gre/modules/devtools/escodegen/escodegen", > + "escodegen/package.json": "resource://gre/modules/devtools/escodegen/package.json", > + "estraverse": "resource://gre/modules/devtools/escodegen/estraverse", > + looks fine to me, but you should r? dcamp for loader changes. ::: toolkit/devtools/escodegen/UPGRADING.md @@ +10,5 @@ > + > +2. Make sure that all tests pass: > + > + $ npm install . > + $ npm test does this run the tests in Firefox or just node? I you can figure out how to update these to run in Firefox (stand-alone, not automated), that'd be sweet.

Comment on attachment 796083 [details] [diff] [review] integrate-escodegen.patch Review of attachment 796083 [details] [diff] [review]: ----------------------------------------------------------------- Looks good, a few changes needed below. I think adding a directory is enough to require build peer review according to the new rules. I'm feedback+'ing, but flag :gps after making the moz.build change below. ::: toolkit/devtools/Loader.jsm @@ +42,5 @@ > } > > // Used when the tools should be loaded from the Firefox package itself (the default) > var BuiltinProvider = { > load: function(done) { You'll need to replicate the changes made to the BuiltinProvider to the srcdir provider (sorry). ::: toolkit/devtools/escodegen/moz.build @@ +3,5 @@ > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +TEST_DIRS += ['tests'] You can add: ---- JS_MODULES_PATH = 'modules/devtools/escodegen' EXTRA_JS_MODULES += [ 'escodegen.js', 'extraverse.js', 'package.json.js' ] --- To your moz.build, and then since that's all Makefile.in is doing in this directory you can remove it entirely. ::: toolkit/devtools/sourcemap/Makefile.in @@ +13,5 @@ > include $(topsrcdir)/config/rules.mk > > libs:: > $(NSINSTALL) $(srcdir)/*.jsm $(FINAL_TARGET)/modules/devtools > + $(NSINSTALL) $(srcdir)/*.js $(FINAL_TARGET)/modules/devtools I guess this won't be needed if source-map.js goes away? ::: toolkit/devtools/sourcemap/source-map.js @@ +1,4 @@ > +const { Cu } = require("chrome"); > +const { SourceNode, SourceMapConsumer, SourceMapGenerator } = Cu.import("resource://gre/modules/devtools/SourceMap.jsm", {}); > +exports.SourceNode = SourceNode; > +exports.SourceMapConsumer = SourceMapConsumer; Is this still necessary with the modules hack?

Flagging gps for build system review. Applied dcamp's feedback: * use moz.build instead of makefiles * remove source-map.js file which wasn't needed any longer * add source-map to SrcdirProvider https://tbpl.mozilla.org/?tree=Try&rev=376505973569

(In reply to Rob Campbell [:rc] (:robcee) from comment #7) > ::: toolkit/devtools/escodegen/UPGRADING.md > @@ +10,5 @@ > > + > > +2. Make sure that all tests pass: > > + > > + $ npm install . > > + $ npm test > > does this run the tests in Firefox or just node? I you can figure out how to > update these to run in Firefox (stand-alone, not automated), that'd be sweet. I talked to Constellation on twitter and he seemed receptice to making (most) of the tests runnable in content pages. I doubt we will be able to integrate with our test infrastructure, but at least we can do testing manually every time we update escodegen in our tree. Will take a look into getting this going once the devtools meet up is over. https://github.com/Constellation/escodegen/issues/124

Comment on attachment 796503 [details] [diff] [review] integrate-escodegen.patch Review of attachment 796503 [details] [diff] [review]: ----------------------------------------------------------------- Just the build bits. ::: toolkit/devtools/escodegen/tests/moz.build @@ +5,5 @@ > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +MODULE = 'test_escodegen' > + > +XPCSHELL_TESTS_MANIFESTS += ['unit/xpcshell.ini'] I'd just remove the moz.build from this directory and move the XPCSHELL_TESTS_MANIFESTS into the parent moz.build. No sense to pollute the tree with near empty moz.build files if it can be avoided.