Skip to content
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.lastIndexOf() #483

Conversation

lvidacs
Copy link
Contributor

@lvidacs lvidacs commented Jul 28, 2015

JerryScript-DCO-1.0-Signed-off-by: Laszlo Vidacs [email protected]

@lvidacs
Copy link
Contributor Author

lvidacs commented Jul 28, 2015

The helper function is prepared to handle indexOf() as well, it will be adjusted in a separate step.

{
/* match as long as possible */
ecma_length_t match_len = 0;
lit_utf8_iterator_pos_t stored_original_pos = lit_utf8_iterator_get_pos (&original_it);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we save the iterator itself? Seeking is not a cheap operation.

@dbatyai dbatyai added ecma builtins Related to ECMA built-in routines development Feature implementation labels Jul 28, 2015
@dbatyai dbatyai added this to the ECMA builtins milestone Jul 28, 2015
@lvidacs lvidacs force-pushed the string-prototype-lastindexof branch from 790d120 to 7f758fe Compare July 29, 2015 15:50
@ILyoan ILyoan mentioned this pull request Jul 30, 2015
25 tasks
extern ecma_completion_value_t ecma_builtin_helper_string_prototype_object_index_of (ecma_value_t this_arg,
ecma_value_t arg1,
ecma_value_t arg2,
bool firstIndex);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect indentation

@lvidacs lvidacs force-pushed the string-prototype-lastindexof branch from 7f758fe to a1e0185 Compare July 30, 2015 10:38
@lvidacs
Copy link
Contributor Author

lvidacs commented Jul 30, 2015

@galpeter, thanks for the comments, the patch is rebased and updated.

if (match_len == search_len)
{
*ret_num_p = ecma_uint32_to_number (index);
found = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the found at all? Can't we just break out?

JerryScript-DCO-1.0-Signed-off-by: Laszlo Vidacs [email protected]
@lvidacs lvidacs force-pushed the string-prototype-lastindexof branch from a1e0185 to 11a6a50 Compare July 30, 2015 13:19
@galpeter
Copy link
Contributor

lgtm

@zherczeg
Copy link
Member

looks good, but please open an issue that where the followup work is described

@lvidacs
Copy link
Contributor Author

lvidacs commented Aug 4, 2015

Issue #515 is opened for follow-up work.

@zherczeg zherczeg assigned egavrin and unassigned lvidacs Aug 4, 2015
@egavrin
Copy link
Contributor

egavrin commented Aug 5, 2015

make push

@egavrin egavrin assigned lvidacs and unassigned egavrin Aug 5, 2015
@kkristof
Copy link
Contributor

kkristof commented Aug 6, 2015

Landed in 554305d

@kkristof kkristof closed this Aug 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Feature implementation ecma builtins Related to ECMA built-in routines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants