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: fix two [[FooIteratorNextIndex]] descriptions #1784

Closed
wants to merge 1 commit into from

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented Nov 15, 2019

... that refer to the index of an index rather than the index of an element.

[Cherry-picked from PR #1623.]

... that refer to the index of an index
rather than the index of an element.

[Cherry-picked from PR tc39#1623.]
@@ -30296,7 +30296,7 @@ <h1>Properties of String Iterator Instances</h1>
[[StringIteratorNextIndex]]
</td>
<td>
The integer index of the next string index to be examined by this iteration.
The integer index of the next string element (code unit) to be examined by this iteration.
Copy link
Member

Choose a reason for hiding this comment

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

I am most certainly wrong, but doesn't StringIterator go over code points?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's true that the next() function returns a code-point-worth of string, but [[StringIteratorNextIndex]] remembers its position as an ordinary (code-unit-wise) index into the string.

(If it remembered its position as a code-point offset, each call to next() would have to re-scan the string to know what to look at next.)

Copy link
Member

Choose a reason for hiding this comment

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

Thank you for explanation. I believe #1579 can possibly supersede this change?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed it can! (Sorry, I wasn't paying attention.)

In the likely event that it gets merged, I'll close this PR.

@ljharb ljharb requested review from michaelficarra, syg, bakkot and a team November 18, 2019 21:54
@ljharb ljharb self-assigned this Nov 18, 2019
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Nov 18, 2019
This one's my fault: during review of PR tc39#1464, I suggested a change to (what is now) the "Parse" line, without noticing that it stranded the "Otherwise" line.
@bakkot
Copy link
Contributor

bakkot commented Nov 18, 2019

Subsumed by #1579, I believe, which was just merged.

@bakkot bakkot closed this Nov 18, 2019
@jmdyck jmdyck deleted the FooIteratorNextIndex branch November 19, 2019 01:03
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.

4 participants