Skip to content

Conversation

@Achie72
Copy link
Member

@Achie72 Achie72 commented Aug 3, 2020

Based on: https://tc39.es/ecma262/#sec-string.prototype.trim
JerryScript-DCO-1.0-Signed-off-by: Bela Toth [email protected]

@Achie72 Achie72 force-pushed the trimFunctions branch 8 times, most recently from 7c925fd to c183a90 Compare August 5, 2020 16:09
ECMA_TRIM_STRING_START = (1u << 0), /**< Trim only from the start */
ECMA_TRIM_STRING_END = (1u << 1), /**< Trim only from the end */
ECMA_TRIM_STRING_STARTEND = (1u << 2), /**< Trim from both start and end */
} ecma_string_trim_flags_t;
Copy link
Member

Choose a reason for hiding this comment

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

No need for these, just call ecma_string_trim_front/ecma_string_trim_back where necessary, and change ecma_string_trim_helper to call both, and be always inline.

@Achie72 Achie72 force-pushed the trimFunctions branch 5 times, most recently from 7706e95 to bb4cf69 Compare August 11, 2020 13:23
@Achie72 Achie72 requested a review from dbatyai August 11, 2020 13:41
@Achie72 Achie72 force-pushed the trimFunctions branch 2 times, most recently from 0765ed3 to 6e4415b Compare August 17, 2020 05:56
Copy link
Member

@rerobika rerobika left a comment

Choose a reason for hiding this comment

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

LGTM

@rerobika rerobika requested a review from dbatyai August 18, 2020 11:31
@rerobika rerobika added the ES.next Related to ES2015+ features label Aug 18, 2020
Comment on lines 2487 to 2488
* - ecma_builtin_global_object_parse_int
* - ecma_builtin_global_object_parse_float
Copy link
Contributor

Choose a reason for hiding this comment

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

parseInt and parseFloat should only use trimStart by spec

Copy link
Member Author

Choose a reason for hiding this comment

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

This is a nice catch i did not look into. Although roughly looking at them this should not alter as the code checks for digits when iterating and in the end, the ecma_utf8_string_to_number call the whole trim in itself as well. I'm looking into our test suites to check for fails if i modify this.

Copy link
Member

@dbatyai dbatyai left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ES.next Related to ES2015+ features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants