Normative: Add String.prototype.replaceAll #2009

mathiasbynens · 2020-05-24T10:38:16Z

Proposal: https://github.com/tc39/proposal-string-replaceall

Note that the changes to String.prototype.matchAll that were part of this proposal have already been merged here: #1716.

spec.html

mathiasbynens · 2020-05-25T05:44:08Z

Thanks for the reviews! I've addressed the feedback.

spec.html

jmdyck

LGTM

syg

lgtm with small nits below

spec.html

gibson042 · 2020-06-01T15:55:18Z

spec.html

+              1. Assert: Type(_replaceValue_) is String.
+              1. Let _captures_ be a new empty List.
+              1. Let _replacement_ be ! GetSubstitution(_searchString_, _string_, _position_, _captures_, *undefined*, _replaceValue_).
+            1. Let _stringSlice_ be the substring of _string_ consisting of the code units from _endOfLastMatch_ (inclusive) up to _position_ (exclusive).


Consistency suggestion (cf. #2021):

Suggested change

1. Let _stringSlice_ be the substring of _string_ consisting of the code units from _endOfLastMatch_ (inclusive) up to _position_ (exclusive).

1. Let _stringSlice_ be the substring of _string_ consisting of the code units at indices _endOfLastMatch_ (inclusive) through _position_ (exclusive).

Note that the notation in #2021 is still in flux and no longer matches the suggestion.

What should I do here? Happy to make any changes to get this to a land-able state.

If we're going to land it before #2021, it doesn't matter what we do here.

edit: To clarify, you should just leave it as it is.

Seems like this one will be able to land before #2021, but I think whichever one lands first should rebase and ensure this prose is consistent?

@ljharb This PR is consistent with existing forms currently.

Seems like it should stay that way then, until #2021 is merged.

spec.html

gibson042

Looks good!

gibson042 · 2020-06-03T08:23:37Z

spec.html

+          1. Let _advanceBy_ be max(1, _searchLength_).
+          1. Let _matchPositions_ be a new empty List.
+          1. Let _position_ be ! StringIndexOf(_string_, _searchString_, 0).
+          1. Repeat, while _position_ is not -1,


Your test in String.prototype.includes is If _index_ ≥ 0. I don't have a preference for "≥ 0" vs. "is not -1", but I do have a preference for them to match.

I've made the change. This is a little awkward though:

If index is not -1, return true. Return false.

In code you'd probably write it as:

If index is -1, return false. Return true.

🤷

The upstream spec PR (tc39/ecma262#2009) still needs to be merged, but we can already update this repository.

Proposal: https://github.com/tc39/proposal-string-replaceall Note that the changes to String.prototype.matchAll that were part of this proposal have already been merged here: tc39#1716. Co-authored-by: Jakob Gruber <jgruber@chromium.org> Co-authored-by: Mathias Bynens <mathias@chromium.org>

PR tc39#2009 recently introduced StringIndexOf, locating its definition in a subclause of "10 ECMAScript Language: Source Code". This placement doesn't make much sense, as StringIndexOf isn't particularly related to source code. This commit relocates it to 6.1.4 The String Type.

mathiasbynens added needs consensus This needs committee consensus before it can be eligible to be merged. has test262 tests labels May 24, 2020

mathiasbynens commented May 24, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

mathiasbynens mentioned this pull request May 24, 2020

Advance to stage 4 tc39/proposal-string-replaceall#28

Closed

mathiasbynens force-pushed the add-string-prototype-replaceall branch 2 times, most recently from 1f53ecb to 6353c08 Compare May 24, 2020 12:33

jmdyck requested changes May 24, 2020

View reviewed changes

ljharb reviewed May 24, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

spec.html Show resolved Hide resolved

spec.html Outdated Show resolved Hide resolved

mathiasbynens force-pushed the add-string-prototype-replaceall branch from 6353c08 to bf8244e Compare May 25, 2020 05:43

mathiasbynens force-pushed the add-string-prototype-replaceall branch from bf8244e to fbc3b9d Compare May 25, 2020 05:44

ljharb reviewed May 25, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

spec.html Show resolved Hide resolved

spec.html Outdated Show resolved Hide resolved

spec.html Outdated Show resolved Hide resolved

ljharb requested a review from jmdyck May 25, 2020 06:01

mathiasbynens force-pushed the add-string-prototype-replaceall branch 3 times, most recently from e221f20 to 4d5efbd Compare May 25, 2020 06:32

ljharb approved these changes May 25, 2020

View reviewed changes

ljharb requested review from michaelficarra, syg, bakkot and a team May 25, 2020 06:36

jmdyck requested changes May 25, 2020

View reviewed changes

spec.html Show resolved Hide resolved

mathiasbynens force-pushed the add-string-prototype-replaceall branch 2 times, most recently from 4933c0e to 9c06d72 Compare May 25, 2020 15:38

mathiasbynens requested a review from jmdyck May 25, 2020 15:38

jmdyck approved these changes May 25, 2020

View reviewed changes

syg approved these changes May 27, 2020

View reviewed changes

spec.html Show resolved Hide resolved

spec.html Outdated Show resolved Hide resolved

michaelficarra reviewed May 28, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

michaelficarra reviewed May 28, 2020

View reviewed changes

spec.html Show resolved Hide resolved

michaelficarra approved these changes May 28, 2020

View reviewed changes

ljharb self-assigned this May 28, 2020

mathiasbynens force-pushed the add-string-prototype-replaceall branch from 9c06d72 to f2286b3 Compare May 28, 2020 06:48

michaelficarra reviewed May 28, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

mathiasbynens force-pushed the add-string-prototype-replaceall branch from f2286b3 to a3e2f2b Compare May 29, 2020 06:07

bakkot reviewed May 31, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

mathiasbynens force-pushed the add-string-prototype-replaceall branch from a3e2f2b to e964506 Compare June 1, 2020 15:11

gibson042 reviewed Jun 1, 2020

View reviewed changes

ljharb added has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels Jun 2, 2020

mathiasbynens requested a review from bakkot June 2, 2020 15:45

mathiasbynens force-pushed the add-string-prototype-replaceall branch from e964506 to 42b11de Compare June 2, 2020 16:04

mathiasbynens requested a review from gibson042 June 3, 2020 08:03

gibson042 approved these changes Jun 3, 2020

View reviewed changes

mathiasbynens force-pushed the add-string-prototype-replaceall branch from 42b11de to 90d82a2 Compare June 3, 2020 11:10

mathiasbynens mentioned this pull request Jun 27, 2020

Advance to stage 4 tc39/proposal-string-replaceall#38

Merged

mathiasbynens pushed a commit to tc39/proposal-string-replaceall that referenced this pull request Jun 27, 2020

Advance to stage 4 (#38)

0dd32b2

The upstream spec PR (tc39/ecma262#2009) still needs to be merged, but we can already update this repository.

ljharb force-pushed the add-string-prototype-replaceall branch from 90d82a2 to 7ccc226 Compare June 27, 2020 21:36

ljharb merged commit 7ccc226 into tc39:master Jun 27, 2020

jmdyck mentioned this pull request Jun 29, 2020

Editorial: Relocate the definition of StringIndexOf #2069

Merged

mathiasbynens deleted the add-string-prototype-replaceall branch June 29, 2020 04:43

ljharb mentioned this pull request Dec 28, 2020

Editorial: reword StringIndexOf in terms of substring #2258

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Add String.prototype.replaceAll #2009

Normative: Add String.prototype.replaceAll #2009

mathiasbynens commented May 24, 2020

mathiasbynens commented May 25, 2020

jmdyck left a comment

syg left a comment

gibson042 Jun 1, 2020

michaelficarra Jun 1, 2020

mathiasbynens Jun 2, 2020

michaelficarra Jun 2, 2020 •

edited

ljharb Jun 2, 2020

michaelficarra Jun 2, 2020

ljharb Jun 2, 2020

gibson042 left a comment

gibson042 Jun 3, 2020

mathiasbynens Jun 3, 2020

	1. Let _stringSlice_ be the substring of _string_ consisting of the code units from _endOfLastMatch_ (inclusive) up to _position_ (exclusive).
	1. Let _stringSlice_ be the substring of _string_ consisting of the code units at indices _endOfLastMatch_ (inclusive) through _position_ (exclusive).

Normative: Add String.prototype.replaceAll #2009

Normative: Add String.prototype.replaceAll #2009

Conversation

mathiasbynens commented May 24, 2020

mathiasbynens commented May 25, 2020

jmdyck left a comment

Choose a reason for hiding this comment

syg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra Jun 2, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gibson042 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelficarra Jun 2, 2020 •

edited