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: Remove unnecessary statement following heading for find. #2294

Closed
wants to merge 2 commits into from

Conversation

DanielRosenwasser
Copy link
Member

I found this while reading over the proposed spec text for findLast. It seems like no other function/method contains text like this.

@bakkot
Copy link
Contributor

bakkot commented Feb 4, 2021

There's a few categories of these, it looks like:

  • .concat/.every/.filter/.findIndex/.flat/.flatMap/.forEach/.indexOf/.lastIndexOf/.map/.push/.reduce/.reduceRight/.some/.unshift have "When the X method is called with [arguments], [optional description]. The following steps are taken:"
    • as a variant, .includes does not include the "with [arguments]" part, despite taking an argument
  • .slice and .splice have a bare "The following steps are taken:"
  • .copyWithin/.fill have "The X method takes up to X arguments A, B, and C." [yes, there is no punctuation after "arguments"]
  • .find has both "The find method is called with one or two arguments, predicate and thisArg." and "When the find method is called, the following steps are taken:"
  • .join has "The join method takes one argument, separator, and performs the following steps:"

(I skipped a few.) And that's just the Array prototype methods; I imagine there's other variants lurking out there. At a quick skim, most do seem to use either the "When the X is called" form or a bare "The following steps are taken:".

Clearly there's more work to do. Ideally someday we'd have something along the lines of #1914 / #1994 / #2253, at least where it makes sense to do so.


Concretely, I think for this PR it would be good to additionally reword the sentence following the NOTE to add "with one or two arguments", as in "When the find method is called with one or two arguments, the following steps are taken:", which will make it match most of the rest of the Array prototype methods.

@DanielRosenwasser
Copy link
Member Author

I think we're good to go now.

@ljharb ljharb requested review from syg, michaelficarra, bakkot and a team March 10, 2021 19:28
@michaelficarra
Copy link
Member

michaelficarra commented Mar 11, 2021

I don't know why we're bothering with this. We're going to just remove all this nonsense from the Array.prototype section anyway. I'll send a PR.

edit: #2346

@bakkot
Copy link
Contributor

bakkot commented Mar 22, 2021

Thanks for kicking this off, @DanielRosenwasser. I believe it's now addressed a little more comprehensively by #2346, so closing this, but feel free to reopen if you think there's more to do here.

@bakkot bakkot closed this Mar 22, 2021
@DanielRosenwasser DanielRosenwasser deleted the findPhrasing branch March 22, 2021 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants