Skip to content
This repository has been archived by the owner on Feb 19, 2021. It is now read-only.

Add minimal specification #13

Closed
wants to merge 4 commits into from
Closed

Add minimal specification #13

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
74e5149
Add minimal specification
mathiasbynens Mar 5, 2019
0142ad5
Remove explicit IsCallable check
mathiasbynens Mar 5, 2019
f76b9d9
Handle the case where stringValue is null or undefined
mathiasbynens Mar 5, 2019
ea69b5a
Update spec.html
ljharb Mar 5, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 12 additions & 18 deletions spec.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,25 +28,19 @@ <h1>String.prototype.replaceAll ( _searchValue_, _replaceValue_ )</h1>
<p>When the `replaceAll` method is called with arguments _searchValue_ and _replaceValue_, the following steps are taken:</p>
<emu-alg>
1. Let _O_ be ? RequireObjectCoercible(*this* value).
1. If _searchValue_ is neither *undefined* nor *null*, then
1. Let _replacer_ be ? GetMethod(_searchValue_, @@replace).
1. If _replacer_ is not *undefined*, then
1. Return ? Call(_replacer_, _searchValue_, &laquo; _O_, _replaceValue_ &raquo;).
1. Let _string_ be ? ToString(_O_).
1. Let _searchString_ be ? ToString(_searchValue_).
1. Let _functionalReplace_ be IsCallable(_replaceValue_).
1. If _functionalReplace_ is *false*, then
1. Set _replaceValue_ to ? ToString(_replaceValue_).
1. Search _string_ for the first occurrence of _searchString_ and let _pos_ be the index within _string_ of the first code unit of the matched substring and let _matched_ be _searchString_. If no occurrences of _searchString_ were found, return _string_.
1. If _functionalReplace_ is *true*, then
1. Let _replValue_ be ? Call(_replaceValue_, *undefined*, &laquo; _matched_, _pos_, _string_ &raquo;).
1. Let _replStr_ be ? ToString(_replValue_).
1. Let _isRegExp_ be ? IsRegExp(_searchValue_).
1. If _isRegExp_ is *true*, then
1. Let _flags_ be _searchValue_.[[OriginalFlags]].
1. If _flags_ does not contain `"g"`, then
1. Set _flags_ to a copy of _flags_.
Copy link
Member

Choose a reason for hiding this comment

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

Why is this needed? This is a value, not a reference to the internal slot on searchValue.

Alternatively, there’s an existing form of “to the concatenation of a and b” that’s probably better here.

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 value, not a reference to the internal slot on searchValue.

That’s not what https://tc39.github.io/ecma262/#sec-algorithm-conventions says:

Algorithm steps may declare named aliases for any value using the form “Let x be someValue”. These aliases are reference-like in that both x and someValue refer to the same underlying data and modifications to either are visible to both. Algorithm steps that want to avoid this reference-like behaviour should explicitly make a copy of the right-hand side: “Let x be a copy of someValue” creates a shallow copy of someValue.

Copy link
Member

Choose a reason for hiding this comment

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

Interesting, that seems terrifying :-) either way i think the concatenation language would be simpler (it’s what matchAll uses)

1. Append `"g"` to _flags_.
1. Else,
1. Let _captures_ be a new empty List.
1. Let _replStr_ be GetSubstitution(_matched_, _string_, _pos_, _captures_, *undefined*, _replaceValue_).
1. Let _tailPos_ be _pos_ + the number of code units in _matched_.
1. Let _newString_ be the string-concatenation of the first _pos_ code units of _string_, _replStr_, and the trailing substring of _string_ starting at index _tailPos_. If _pos_ is 0, the first element of the concatenation will be the empty String.
1. Return _newString_.
1. Let _searchString_ be ? ToString(_searchValue_).
1. Set _escapedPattern_ to EscapeRegExpPattern(_src_, `"g"`).
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
1. Set _escapedPattern_ to EscapeRegExpPattern(_src_, `"g"`).
1. Set _escapedPattern_ to EscapeRegExpPattern(_searchString_, `"g"`).

1. Set _searchValue_ to ? RegExpCreate(_escapedPattern_, `"g"`).
Copy link
Contributor

Choose a reason for hiding this comment

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

So... We add g flag and don't create a copy of regexp with required flags, just use original regexp?

1. Assert: ! IsRegExp(_searchValue_) is *true*.
1. Let _replacer_ be ? GetMethod(_searchValue_, @@replace).
1. Return ? Call(_replacer_, _searchValue_, &laquo; _O_, _replaceValue_ &raquo;).
Copy link
Member

Choose a reason for hiding this comment

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

this could also just call Invoke directly?

</emu-alg>
<emu-note>
<p>The `replaceAll` function is intentionally generic; it does not require that its *this* value be a String object. Therefore, it can be transferred to other kinds of objects for use as a method.</p>
Expand Down