Reviewers: Søren Gjesse, Message: My first patch to v8, please help me correct any mistakes.
Mr. Gjesse, I just guessed that you are the reviewer, because you've been a reviewer on past changes to the shell sample. Please let me know if someone else is more appropriate to review this change. Here's a little history behind the patch: I was using the V8 sample shell to run benchmarks from http://shootout.alioth.debian.org/, and I noticed that many of the JavaScript benchmarks assumed that command-line-shell arguments would be available to the JavaScript script in the "arguments" global variable. This is apparently a standard among JavaScript shells. I also notice the topic was raised a few months ago on the v8-users mailing list, but the suggested work-around was for each developer to modify their own copy of the shell sample to add argument parsing. That didn't sound right to me. If it's a standard expected feature of JavaScript shells, there ought to be standard support for it in v8. While researching how to implement this feature I discovered that it was already mostly implemented. The V8::SetFlagsFromCommandLine function already recognized the "--" flag and saved away the arguments. All that was missing was a public way of retrieving the saved arguments, and some utility code to convert them to a JavaScript array and install them as a global variable. I found some code that implemented this in the d8 shell. So I took the liberty of extracting that code and making it a public API, which is now called from both the shell sample and the d8 shell. Description: Add the "arguments" global to the shell sample. The "arguments" global is commonly used by Javascript shells to pass command line arguments to the Javascript script. Rhino does this, as does v8's own "d8" shell. This change works by adding a new public API to v8, GetJSAttributes(), which returns a Javascript array of the arguments. While it might seem scary to add a new API, this API is already implicit in the way that arguments are parsed by the existing SetFlagsFromString and SetFlagsFromCommandLine APIs. Those APIs already recognize the "--" flag and saves away any arguments after the "--" flag. All this change does is prove a way of getting at the saved arguments. The implementation of GetJSAttributes() was extracted from the d8 shell. Please review this at http://codereview.chromium.org/99254 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M include/v8.h M samples/shell.cc M src/api.cc M src/d8.cc Index: include/v8.h =================================================================== --- include/v8.h (revision 1828) +++ include/v8.h (working copy) @@ -1977,6 +1977,11 @@ char** argv, bool remove_flags); + /** + * Get the js_arguments from the V8 flags. + */ + static Local<Array> GetJSArguments(); + /** Get the version string. */ static const char* GetVersion(); Index: samples/shell.cc =================================================================== --- samples/shell.cc (revision 1828) +++ samples/shell.cc (working copy) @@ -61,6 +61,13 @@ global->Set(v8::String::New("quit"), v8::FunctionTemplate::New(Quit)); // Bind the 'version' function global->Set(v8::String::New("version"), v8::FunctionTemplate::New(Version)); + // Create a utility context to allow us to create an array + v8::Handle<v8::Context> utility_context_ = v8::Context::New(NULL, global); + utility_context_->SetSecurityToken(v8::Undefined()); + v8::Context::Scope utility_scope(utility_context_); + // Bind the global 'arguments' variable to the "--" arguments from the command + // line. + global->Set(v8::String::New("arguments"), v8::V8::GetJSArguments()); // Create a new execution environment containing the built-in // functions v8::Handle<v8::Context> context = v8::Context::New(NULL, global); Index: src/api.cc =================================================================== --- src/api.cc (revision 1828) +++ src/api.cc (working copy) @@ -261,6 +261,19 @@ i::FlagList::SetFlagsFromCommandLine(argc, argv, remove_flags); } +Local<Array> V8::GetJSArguments() { + i::JSArguments js_args = i::FLAG_js_arguments; + i::Handle<i::FixedArray> arguments_array = + i::Factory::NewFixedArray(js_args.argc()); + for (int j = 0; j < js_args.argc(); j++) { + i::Handle<i::String> arg = + i::Factory::NewStringFromUtf8(i::CStrVector(js_args[j])); + arguments_array->set(j, *arg); + } + i::Handle<i::JSArray> arguments_jsarray = + i::Factory::NewJSArrayWithElements(arguments_array); + return Utils::ToLocal(arguments_jsarray); +} v8::Handle<Value> ThrowException(v8::Handle<v8::Value> value) { if (IsDeadCheck("v8::ThrowException()")) return v8::Handle<Value>(); Index: src/d8.cc =================================================================== --- src/d8.cc (revision 1828) +++ src/d8.cc (working copy) @@ -412,18 +412,8 @@ utility_context_->SetSecurityToken(Undefined()); Context::Scope utility_scope(utility_context_); - i::JSArguments js_args = i::FLAG_js_arguments; - i::Handle<i::FixedArray> arguments_array = - i::Factory::NewFixedArray(js_args.argc()); - for (int j = 0; j < js_args.argc(); j++) { - i::Handle<i::String> arg = - i::Factory::NewStringFromUtf8(i::CStrVector(js_args[j])); - arguments_array->set(j, *arg); - } - i::Handle<i::JSArray> arguments_jsarray = - i::Factory::NewJSArrayWithElements(arguments_array); global_template->Set(String::New("arguments"), - Utils::ToLocal(arguments_jsarray)); + V8::GetJSArguments()); #ifdef ENABLE_DEBUGGER_SUPPORT // Install the debugger object in the utility scope --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
