Closed
Bug 1065180
Opened 10 years ago
Closed 10 years ago
Replace the JS shell's environment object with a getenv() function
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: jorendorff, Assigned: jorendorff)
Details
Attachments
(3 files, 1 obsolete file)
3.23 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
4.66 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
5.48 KB,
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
Assignee: nobody → jorendorff
Attachment #8486864 -
Flags: review?(sphink)
Assignee | ||
Comment 2•10 years ago
|
||
Attachment #8486871 -
Flags: review?(sphink)
Assignee | ||
Comment 3•10 years ago
|
||
Attachment #8486872 -
Flags: review?(sphink)
Assignee | ||
Comment 4•10 years ago
|
||
This time with the .cpp file.
Attachment #8486864 -
Attachment is obsolete: true
Attachment #8486864 -
Flags: review?(sphink)
Attachment #8487200 -
Flags: review?(sphink)
Comment 5•10 years ago
|
||
Comment on attachment 8487200 [details] [diff] [review] Part 1 - Add os.getenv(). Review of attachment 8487200 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/shell/OSObject.cpp @@ +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/. */ > + > +// OSObject.h - os object for exposing posix system calls as needed in the JS shell "as needed"? Needed by what? s/as needed// @@ +12,5 @@ > + > +using namespace JS; > + > +static bool > +os_getenv(JSContext* cx, unsigned argc, JS::Value* vp) This is the only place you used JS::. Though come to think of it, I don't have a problem with that. Value sounds pretty generic. @@ +17,5 @@ > +{ > + CallArgs args = CallArgsFromVp(argc, vp); > + RootedString key(cx, ToString(cx, args.get(0))); > + if (!key) > + return false; So this looks up $undefined if you don't pass the arg? Seems weird. @@ +46,5 @@ > + RootedObject obj(cx, JS_NewObject(cx, nullptr, JS::NullPtr(), JS::NullPtr())); > + return obj && > + JS_DefineFunctions(cx, obj, os_functions) && > + JS_DefineProperty(cx, global, "os", obj, 0); > +} This whole file is short and sweet. I like it. ::: js/src/shell/OSObject.h @@ +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/. */ > + > +// OSObject.h - os object for exposing posix system calls as needed in the JS shell s/as needed//
Attachment #8487200 -
Flags: review?(sphink) → review+
Updated•10 years ago
|
Attachment #8486871 -
Flags: review?(sphink) → review+
Updated•10 years ago
|
Attachment #8486872 -
Flags: review?(sphink) → review+
Assignee | ||
Comment 6•10 years ago
|
||
(In reply to Steve Fink [:sfink] from comment #5) > > +// OSObject.h - os object for exposing posix system calls as needed in the JS shell > > "as needed"? Needed by what? s/as needed// Heh. OK. For the record, the answer to "Needed by what?" was clear enough to me: the handful of shell users we care about. > > + CallArgs args = CallArgsFromVp(argc, vp); > > + RootedString key(cx, ToString(cx, args.get(0))); > > + if (!key) > > + return false; > > So this looks up $undefined if you don't pass the arg? Seems weird. Hmm. I guess that is egregious enough that I should have it throw an error instead.
Assignee | ||
Comment 7•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5921f2808ade https://hg.mozilla.org/integration/mozilla-inbound/rev/4b19eda14b04 https://hg.mozilla.org/integration/mozilla-inbound/rev/ba05f247d98c
Comment 8•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/5921f2808ade https://hg.mozilla.org/mozilla-central/rev/4b19eda14b04 https://hg.mozilla.org/mozilla-central/rev/ba05f247d98c
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee | ||
Comment 9•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/5d3aca3b7383
You need to log in
before you can comment on or make changes to this bug.
Description
•