Make 'traceur' cleanup process.argv and setup the node.js module so that relative require works. (issue 7552046)

[a...@chromium.org]

I'm not sure why we need to do the process.argv manipulation in
command.js?

Also, I need you to fill out AUTHORS and sign the CLA. See th e wiki for
link to the CLA:
https://code.google.com/p/traceur-compiler/wiki/Contributions


https://codereview.appspot.com/7552046/diff/1/src/node/command.js
File src/node/command.js (right):

https://codereview.appspot.com/7552046/diff/1/src/node/command.js#newcode94
src/node/command.js:94: process.argv = process.argv.slice(0,
1).concat(includes);
The idea was that interpreter.js would do this.

https://codereview.appspot.com/7552046/diff/1/src/node/interpreter.js
File src/node/interpreter.js (right):

https://codereview.appspot.com/7552046/diff/1/src/node/interpreter.js#newcode27
src/node/interpreter.js:27: require.main.filename = filename;
makes sense

https://codereview.appspot.com/7552046/

[a...@chromium.org]

Sorry, I should have looked closer the first time. I added some comments
on the bug.


https://codereview.appspot.com/7552046/diff/6001/src/node/interpreter.js
File src/node/interpreter.js (left):

https://codereview.appspot.com/7552046/diff/6001/src/node/interpreter.js#oldcode28
src/node/interpreter.js:28: argv[0] = 'traceur';
The idea was that argv would work like without traceur where it replaces
'node' with 'traceur'

https://codereview.appspot.com/7552046/

[usrb...@yahoo.com]

Note: I'm mostly playing the role of opinionated observer here. arv is
the main reviewer here.

Thanks for getting to this bug. It's one that I've had my eye on for
awhile, but haven't been able to get to.

The first patch (maybe second or third, actually) is the hardest, so
please stick with it.

A full fix for the arg-handling bugs would address all the errors in my
later comment. I think the arg fix works in more cases than our current
code, but it still has problems. If this code is accepted, there needs
to be a follow-up eventually.

The 'require.main.filename' fix seems fine from the brief testing I did.
I'm not too familiar with node's magic internals.

https://codereview.appspot.com/7552046/diff/1/src/node/command.js
File src/node/command.js (right):

https://codereview.appspot.com/7552046/diff/1/src/node/command.js#newcode94
src/node/command.js:94: process.argv = process.argv.slice(0,
1).concat(includes);

> interpreter.js doesn't know which arguments are flags and where actual
> arguments start for the script running under traceur. includes is the
> array containing all the arguments following traceur options.

There are actually a host of bugs in the arg handling code. Most of them
have to do with the Commander.js library we use for arg handling.

#--cut--
## try this code with the unpatched version. You can see what's going on
## more clearly, since argv is only slightly modified.

cat > argv.js <<"END"
console.log(process.argv) // no semicolon on purpose
END

## bug. should interpret
./traceur argv.js --out argv.out.js

## bug. should not process args following filename.
./traceur argv.js --strict-semicolons

## let's see what 'includes' contains.
sed -e '/var includes =/a\
console.log("includes: %s", includes); // REMOVEME' \
-i src/node/command.js

## 'includes' is actually just the leftover pieces that aren't attached
## to an option or something that looks like an option.
./traceur argv.js --some-random-option file1.js file2.js

## remove our debug code.
sed -e '/REMOVEME/d' -i src/node/command.js
#--cut--

See the discussion here for the previous discussion about these bugs:

Command line traceur will now invoke the interpreter if no out param
is passed
https://codereview.appspot.com/7810043

The options (no pun intended) are to patch the upstream library to allow
the behavior we want, or to write our own code. I'm personally leaning
towards trying to patch upstream, since my first github pull request
went so well.

Update. After looking through the Commander.js code, it seems
needlessly complex with a lot of things we don't use. I'm still
leaning towards it, but it's more out of a sense of contributing to a
library used by many, rather than getting from point A to point B in
the shortest time.

https://codereview.appspot.com/7552046/diff/1/src/node/command.js#newcode94
src/node/command.js:94: process.argv = process.argv.slice(0,
1).concat(includes);

On 2013/03/27 14:39:52, arv-chromium wrote:
> The idea was that interpreter.js would do this.

You know, I wonder if something in the spirit of 'execv'

int execv(const char *path, char *const argv[]);

might not divide the responsibility better. I mean, that's essentially
what 'interpret' does. At least in effect, even if not in the details.

// So these would be conceptually equivalent:

// C
char **newArgv = munge(argv);
execv(filename, newArgv);

// JS
var newArgv = munge(process.argv);
interpret('traceur', includes[0], newArgv);

// In this example, 'interpret' would just do a simple
function interpret(interpreter, filename, newArgv) {
...
var argv = [interpreter, filename];
argv.push.apply(argv, newArgv);
process.argv = argv;
...
}

https://codereview.appspot.com/7552046/