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

Editorial: quick fixes #2022

Merged
merged 4 commits into from
Jun 15, 2020
Merged

Editorial: quick fixes #2022

merged 4 commits into from
Jun 15, 2020

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented May 29, 2020

... for recent and not-that-recent merges.

Copy link
Contributor

@bakkot bakkot left a comment

Choose a reason for hiding this comment

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

LGTM, thanks. My comments are for posterity, not suggested changes.

spec.html Outdated
@@ -13156,7 +13156,7 @@ <h1>Runtime Semantics: GetTemplateObject ( _templateLiteral_ )</h1>
1. Let _index_ be 0.
1. Repeat, while _index_ &lt; _count_,
1. Let _prop_ be ! ToString(_index_).
1. Let _cookedValue_ be the String value _cookedStrings_[_index_].
1. Let _cookedValue_ be _cookedStrings_[_index_].
Copy link
Contributor

Choose a reason for hiding this comment

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

For posterity, I want to note that this was correct before #773; it was an oversight that this was not changed as part of this PR.

@@ -13484,15 +13484,11 @@ <h1>Static Semantics: Contains</h1>
</emu-alg>
<emu-grammar>OptionalChain : `?.` IdentifierName</emu-grammar>
<emu-alg>
1. If _symbol_ is a |ReservedWord|, return *false*.
Copy link
Contributor

Choose a reason for hiding this comment

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

Gah, thanks. I noticed this was going to happen as part of the optional chaining PR and then completely forgot about it when #1519 landed.

spec.html Outdated
@@ -27240,7 +27236,7 @@ <h1>Number.prototype.toExponential ( _fractionDigits_ )</h1>
1. Return the string-concatenation of _s_ and _m_.
</emu-alg>
<emu-note>
<p>For implementations that provide more accurate conversions than required by the rules above, it is recommended that the following alternative version of step 10.b.i be used as a guideline:</p>
<p>For implementations that provide more accurate conversions than required by the rules above, it is recommended that the following alternative version of step 9.b.i be used as a guideline:</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Note to self: tc39/ecmarkup#192 needs addressing so this can't happen.

Copy link
Contributor

Choose a reason for hiding this comment

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

See also: tc39/ecmarkdown#56

@ljharb ljharb requested review from syg, ljharb and a team May 30, 2020 20:34
Copy link
Contributor

@syg syg left a comment

Choose a reason for hiding this comment

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

lgtm, thank you for fixes.

@ljharb ljharb self-assigned this Jun 11, 2020
jmdyck added 4 commits June 15, 2020 12:43
…c39#2022)

PR tc39#1938 (among other things) modified the steps of Number.prototype.toExponential, but didn't update the step-reference in the following Note.
)

These steps come from PR tc39#1646, which went through its whole life-cycle while tc39#1519 was dormant. Properly, after tc39#1646 was merged, I should have rebased tc39#1519 and included these changes there.
PR tc39#773 introduced |NotEscapeSequence|, whose TV is *undefined*.
Thus, in the List returned by TemplateStrings with _raw_ == *false*,
any element might be *undefined*.
In GetTemplateObject(), `_cookedStrings_` gets this List,
so when its elements are later extracted via `_cookedStrings_[_index_]`,
the "inline assertion" of `the String value` no longer applies.
@ljharb ljharb merged commit f8e7557 into tc39:master Jun 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants