-
Notifications
You must be signed in to change notification settings - Fork 687
Implement String.prototype.split function #530
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Implement String.prototype.split function #530
Conversation
b27c817 to
704f1df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ecma_get_named_property could be used in the case. The version includes assertion from line 1715.
704f1df to
976500d
Compare
|
I've fixed the mentoined parts. |
|
@bzsolt, thank you! I've got no other comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont see a similar test from the spec:
"A<B>bold</B>and<CODE>coded</CODE>".split(/<(\/)?([^<>]+)>/) ->
["A", undefined, "B", "bold", "/", "B", "and", undefined, "CODE", "coded", "/", "CODE", ""]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right. I've implemented the missing part and added a related testcase (what you wrote).
Thanks.
|
LGTM. |
976500d to
5277461
Compare
|
@ruben-ayrapetyan @sand1k |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't this operation throw an exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should never throw an exception. I've added an extra assert to make sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok. Thank you.
JerryScript-DCO-1.0-Signed-off-by: Zsolt Borbély zsborbely.u-szeged@partner.samsung.com
5277461 to
640370d
Compare
|
I've got no other comments. |
|
lgtm |
JerryScript-DCO-1.0-Signed-off-by: Zsolt Borbély zsborbely.u-szeged@partner.samsung.com