First round of comments.
Also there is the general question of whether these new functions on
String.prototype should be put behind the --harmony flag or not. Personally
I
think they are pretty simple and I don't have a beef with exposing them
right
away, but we need to be cautious. I'll loop in more people in this
discussion.
https://codereview.chromium.org/20928002/diff/1/src/string.js
File src/string.js (right):
https://codereview.chromium.org/20928002/diff/1/src/string.js#newcode894
src/string.js:894: function StringEndsWith(searchString /* position */)
{ // length == 1
nit: Two white-spaces before comment at end of line.
https://codereview.chromium.org/20928002/diff/1/src/string.js#newcode907
src/string.js:907: if (%_ArgumentsLength() > 1) {
There is a missing check as to whether "position" is explicitly passed
an "undefined" value, in which case "pos" should remain s.length here.
Can we add a test-case for that?
https://codereview.chromium.org/20928002/diff/1/src/string.js#newcode909
src/string.js:909: if (!NUMBER_IS_NAN(arg)) {
In case ToNumber(position) yields NaN, then "pos" should be set to 0
IIUC. Better use the internal ToInteger() helper here, that should do
the right thing.
Can we add a test-case for that?
https://codereview.chromium.org/20928002/diff/1/src/string.js#newcode914
src/string.js:914: var end = $Math.min($Math.max(pos, 0), s_len);
This calls potentially monkey-patched versions of Math.min() and
Math.max(), better directly call the MathMin() and MathMax()
implementations directly.
https://codereview.chromium.org/20928002/
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.
