minor style changes
http://codereview.chromium.org/565007/diff/10001/10003 File include/v8.h (right): http://codereview.chromium.org/565007/diff/10001/10003#newcode1357 include/v8.h:1357: Handle<Value> GetScriptOrigin() const; On 2010/02/11 15:29:18, Søren Gjesse wrote:
Please call this function GetScriptName.
When I suggested GetScriptOrign I was thinking of returning a
ScriptOrign C++
object with the same information as was passed to Script::Compile.
Returning a
ScriptOrign object is probably not a good idea after all, as it would
have to be
allocated by V8 and freed by the client. Instead passing a reference
to a
ScriptOrign would make it possible to return the information
void GetScriptOrign(ScriptOrigin* origin);
Used like this:
ScriptOrigin origin; GetScriptOrign(&origin); ... origin.GetResourceName() ...
I think both would be useful, and we can extend the ScriptOrigin with information about whether the script originates from normal compile or
eval,
etc.
Done. http://codereview.chromium.org/565007/diff/10001/10003#newcode1358 include/v8.h:1358: int GetScriptLineNumber() const; On 2010/02/11 15:29:18, Søren Gjesse wrote:
Please add a comment here explaining that the offset in the script
origin is
taken into account and that line numbers start at 0 not 1.
Done. http://codereview.chromium.org/565007/diff/10001/10002 File src/api.cc (right): http://codereview.chromium.org/565007/diff/10001/10002#newcode2450 src/api.cc:2450: if (*func && On 2010/02/11 15:29:18, Søren Gjesse wrote:
Please change *func to !func.IsEmpty().
Done. http://codereview.chromium.org/565007/diff/10001/10002#newcode2469 src/api.cc:2469: return i::GetScriptLineNumber(script, func->shared()->start_position()); On 2010/02/11 15:29:18, Søren Gjesse wrote:
I think this function uses liniar search through the line ends array.
If you
will be calling it a lot using binary search might make a difference.
Yep, I saw that. I think it would be better to push the changes for it as another patch. http://codereview.chromium.org/565007/diff/18013/11013 File include/v8.h (right): http://codereview.chromium.org/565007/diff/18013/11013#newcode1363 include/v8.h:1363: v8::ScriptOrigin GetScriptOrigin() const; On 2010/02/12 16:06:07, Søren Gjesse wrote:
Is v8:: needed?
Done. http://codereview.chromium.org/565007 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
