Skip to content

Conversation

@rwalczyna
Copy link
Contributor

@rwalczyna rwalczyna commented May 13, 2020

Template literals already supports escaping backslash and backtick, this commit enables escaping this in String.raw.

It also fixes invalid behaviour of String.raw when using new line character - escaped new line characters were not included in string.

There was also application crash when escaping backtick in String.raw argument.

JerryScript-DCO-1.0-Signed-off-by: Rafal Walczyna [email protected]

Template literals already supports escaping backslash and backtick,
this commit enables escaping this in String.raw.
It also fixes invalid behaviour of String.raw when using
new line character.

JerryScript-DCO-1.0-Signed-off-by: Rafal Walczyna [email protected]
{
source_p += 3;
#if ENABLED (JERRY_ES2015)
length += 3 * raw_length_inc;
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this raw_length_inc, or can the raw length be computed as source_p - string_start_p. If yes, perhaps we could just remove the variable. This could be more future proof approach.

Copy link
Member

Choose a reason for hiding this comment

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

IMO that way is not completely valid, since in raw strings there are character sequences like (\r\n) which going to be transformed (into \n) therefore a single subtraction is not enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, we could remove raw_length_inc, but we still would need another variable (which also has to be guarded with JERRY_ES2015) to check for \r\n to \n conversions, as rerobika said. Also, source_p points to one character after literal ends when there is $ in string. So length (for raw string) would be source_p - string_start_p - raw_length_dec

@zherczeg I can add it to this commit, there is not so much to change, if you think this would improve performance or code readability :)

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 zherczeg May 19, 2020 08:24
Copy link
Member

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM
I would not mind to improve the code later with source_p - string_start_p - raw_length_dec.

@dbatyai dbatyai merged commit a4659a8 into jerryscript-project:master May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants