Last Comment Bug 720611 - JS shell function to eval with filename
: JS shell function to eval with filename
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86 Mac OS X
: -- normal (vote)
: mozilla12
Assigned To: Dave Herman [:dherman]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-01-23 22:32 PST by Dave Herman [:dherman]
Modified: 2012-01-26 16:21 PST (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
initial implementation (4.12 KB, patch)
2012-01-24 19:39 PST, Dave Herman [:dherman]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description Dave Herman [:dherman] 2012-01-23 22:32:15 PST
The easiest way to shim a CommonJS-like functionality into the JS shell would be to provide a function

    evalAt(src, filename, lineno)

that evaluates source code with a given file name and line number. This would make it possible to wrap CommonJS source code loaded from a file with the necessary CommonJS boilerplate without losing source location information.

Dave
Comment 1 Dave Herman [:dherman] 2012-01-24 19:39:25 PST
Created attachment 591357 [details] [diff] [review]
initial implementation
Comment 2 Dave Herman [:dherman] 2012-01-24 19:48:48 PST
Here's a simple patch. Seems ... not unreasonable. Anyone opposed? I'll probably r? someone tomorrow.

Dave
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-24 23:07:49 PST
Not opposition per se, but why?  The JS shell is basically our own little internal testing repl, not particularly for public use; and slightly a demonstration of good, kosher JSAPI use.  What's the reason we'd be shimming something CommonJSy into it?
Comment 4 Donovan Preston [:fzzzy] 2012-01-25 09:32:11 PST
We run the dom.js test suite with the spidermonkey shell; it sure would be nice to have sane tracebacks. Right now, when you get a traceback that came from a <script> tag inside an html page that dom.js parsed and executed, you get dom.js as the filename and a line number that doesn't exist at all, instead of the url of the html file as the filename and the correct line number to the script inside the html file.
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-25 10:15:40 PST
Ah, that makes sense.  Carry on, Mr. Bowditch.
Comment 6 Dave Herman [:dherman] 2012-01-25 14:01:14 PST
> Not opposition per se, but why?  The JS shell is basically our own little
> internal testing repl, not particularly for public use; and slightly a
> demonstration of good, kosher JSAPI use.  What's the reason we'd be shimming
> something CommonJSy into it?

Narcissus's REPL is built to work with the SpiderMonkey shell, and I want to refactor the modules to be CommonJS so they can also work out-of-the-box in Node.

Dave
Comment 7 Jeff Walden [:Waldo] (remove +bmo to email) 2012-01-25 17:53:38 PST
Comment on attachment 591357 [details] [diff] [review]
initial implementation

Review of attachment 591357 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/shell/js.cpp
@@ +798,5 @@
> +        return false;
> +    }
> +
> +    JSString *code = JS_ValueToString(cx, JS_ARGV(cx, vp)[0]);
> +    if (!code)

You need to have a JS::Anchor<JSString*> anch(code) here, else (depending on the compiler) nothing will root |code|, which could cause its characters to be deallocated underneath you before you're done with them.

@@ +4118,5 @@
>  "revertVersion()          Revert previously set version number",
>  "options([option ...])    Get or toggle JavaScript options",
>  "load(['foo.js' ...])     Load files named by string arguments",
>  "evaluate(code)           Evaluate code as though it were the contents of a file",
> +"evalFrom(code, filename, lineno)\n"

I'd lean toward evalWithLocation as the name.  "from" descriptively just doesn't cut it for me.  If you're especially attached to the current name, tho, feel free to keep using it.
Comment 9 Marco Bonardo [::mak] 2012-01-26 16:21:59 PST
https://hg.mozilla.org/mozilla-central/rev/8d39654512d5

fwiw, the bug number in the commit message is wrong

Note You need to log in before you can comment on or make changes to this bug.