Closed Bug 779038 Opened 9 years ago Closed 9 years ago
add a Evaluate() variant which can read a file
No description provided.
CompileOptions is great!
Assignee: general → bpeterson
Attachment #647439 - Flags: review?(jimb)
Set filename correctly.
Attachment #647441 - Flags: review? → review?(jimb)
Comment on attachment 647441 [details] [diff] [review] add and use in load() Review of attachment 647441 [details] [diff] [review]: ----------------------------------------------------------------- Looks great --- thanks! r=me, with the UTF8 thing fixed. Please consider the "Auto" class and the JS::Compile filename fix as well. ::: js/src/jsapi.cpp @@ +5058,5 @@ > return true; > } > > +static FILE * > +OpenFile(JSContext *cx, const char *filename) You need a comment atop this function: a one-line summary of what it does, and then explaining that "-" is special, and that it might return stdin, which mustn't be closed. You know what would be cool? A class that has a fallible "open" member function, and a destructor that fcloses (or does not fclose) the FILE * as appropriate. Just a suggestion; doesn't need to be in this bug unless you feel like it. We usually give such classes a name beginning with "Auto". @@ +5646,5 @@ > + fclose(fp); > + if (!ok) > + return NULL; > + > + options = options.setUTF8(true).setFileAndLine(filename, 1); The JS:: functions don't assume UTF8. It should be up to the caller to set that if they want it. It's also the case that JS::Compile(..., filename) doesn't set options.filename or options.lineno, but I think that's a bug. JS::Compile(..., filename) and JS::Evaluate(..., filename) should set those in options. Could you fix JS::Compile(..., filename)?
Attachment #647441 - Flags: review?(jimb) → review+
Thanks for the review. https://hg.mozilla.org/integration/mozilla-inbound/rev/668a0ce3f6d7
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla17
You need to log in before you can comment on or make changes to this bug.