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: reword StringIndexOf in terms of substring #2258

Merged
merged 1 commit into from
Jan 6, 2021

Conversation

bakkot
Copy link
Contributor

@bakkot bakkot commented Dec 26, 2020

This is a very small tweak. I find the explicit loop to be easier to understand than the previous "if there exists ... such that for all ..." prose.

Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

This is a massive improvement, thanks

@ljharb ljharb requested review from syg, michaelficarra and a team December 26, 2020 05:20
Copy link
Member

@mathiasbynens mathiasbynens left a comment

Choose a reason for hiding this comment

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

Although git blame points to me here, this was inherited from the old phrasing (from before StringIndexOf existed). I agree this patch makes things much easier to follow!

@ljharb
Copy link
Member

ljharb commented Dec 28, 2020

(added in #2009; i commented on this here :-p )

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.

Very nice.

@ljharb ljharb added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jan 6, 2021
@ljharb ljharb force-pushed the reword-stringindexof branch from dfa8c90 to e656451 Compare January 6, 2021 22:24
@ljharb ljharb merged commit e656451 into master Jan 6, 2021
@ljharb ljharb deleted the reword-stringindexof branch January 6, 2021 22:42
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
ljharb added a commit to ljharb/ecma262 that referenced this pull request Sep 28, 2021
mathiasbynens pushed a commit to mathiasbynens/ecma262 that referenced this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
editorial change ready to merge Editors believe this PR needs no further reviews, and is ready to land.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants