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

String.replace(regexp|substr, newSubstr|function) #116

Closed
jasonwilliams opened this issue Sep 27, 2019 · 5 comments
Closed

String.replace(regexp|substr, newSubstr|function) #116

jasonwilliams opened this issue Sep 27, 2019 · 5 comments
Labels
enhancement New feature or request Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com

Comments

@jasonwilliams
Copy link
Member

replace function would need to be added to the string object here:
https://github.com/jasonwilliams/boa/blob/master/src/lib/js/string.rs

It would be very similar to:
https://github.com/jasonwilliams/boa/blob/master/src/lib/js/regexp.rs#L212-L251

Spec:
https://tc39.es/ecma262/#sec-string.prototype.replace

Notes:
https://github.com/jasonwilliams/boa/blob/master/docs/debugging.md

@jasonwilliams jasonwilliams added enhancement New feature or request good first issue Good for newcomers Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com labels Sep 27, 2019
@jasonwilliams jasonwilliams added this to the Regex milestone Sep 27, 2019
@jasonwilliams jasonwilliams removed the good first issue Good for newcomers label Sep 27, 2019
@louiidev
Copy link

louiidev commented Oct 4, 2019

Hey mate, can I have a go of this?

@thiagoarrais
Copy link

@louisgjohnson: still working on this? I would like to help with it.

@cemremengu
Copy link

cemremengu commented Jul 22, 2020

@jasonwilliams I think this is already added (as I understand from the discussions)

@RageKnify
Copy link
Member

@jasonwilliams you implemented this in #217, but as I noted in #629 , there is a bug with the current implementation:
Copied from Chrome:

> let str = 'boa';
> str.replace(/.*/, "??$&??")
"??boa??"
> str.replace(".*", "??$&??")
"boa"

While on boa both would return "??boa??" because the string ".*" is converted to a RegEx pattern.

Looking at the spec: https://tc39.es/ecma262/#sec-string.prototype.replace
I believe on point 2 if searchValue is a RegExp object it should give RegExp.prototype [ @@replace ] as replacer. Which would then be invoked. @@replace exists, but RegExp.prototype [ @@replace ] isn't implemented, I think the code in String.replace is an aproximation of it.

After #629 gets merged I'd like to look into implementing RegExp.prototype[ @@replace ] and making String.replace spec compliant.

@jedel1043
Copy link
Member

Closing, as the bug that blocked this is solved. If we overlooked something we can open another issue with the specific problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Hacktoberfest Hacktoberfest 2021 - https://hacktoberfest.digitalocean.com
Projects
None yet
Development

No branches or pull requests

6 participants