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: Define each thisFooValue AO in its own section #3063

Merged
merged 2 commits into from
Aug 4, 2023

Conversation

jmdyck
Copy link
Collaborator

@jmdyck jmdyck commented May 10, 2023

Specifically:

  • thisBooleanValue
  • thisSymbolValue
  • thisNumberValue
  • thisBigIntValue
  • thisTimeValue
  • thisStringValue

Since ES6, each of these operations has been defined in the corresponding "Properties of the Foo Prototype Object" section, rather than in a section dedicated to the operation, with a structured header, as is now the norm.

Past efforts in this direction have encountered the problem of placement. E.g., in the status quo, thisBooleanValue is defined within 20.3.3 Properties of the Boolean Prototype Object, so if you were to simply create a subsection for thisBooleanValue right there, context would suggest that thisBooleanValue is a property of the Boolean Prototype Object.

This PR solves that problem by placing each new section within the section for the corresponding valueOf method, which in each case is basically just a call to the thisFooValue operation.

For Symbol and Date, there's an "Abstract Operations" section where the thisFooValue operation could go instead if you wanted. I like the consistency of it always being under the corresponding valueOf.

@bakkot
Copy link
Contributor

bakkot commented May 10, 2023

Should we capitalize the first letter of these for consistency with other AOs while we're at it? I say yes.

@jmdyck
Copy link
Collaborator Author

jmdyck commented May 10, 2023

Ah right, I meant to ask about that.

@ljharb
Copy link
Member

ljharb commented May 10, 2023

That’s a lot of churn for minimal value imo ¯\_(ツ)_/¯

Copy link
Member

@michaelficarra michaelficarra left a comment

Choose a reason for hiding this comment

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

Love it. +1 on Pascal casing AOs.

@jmdyck
Copy link
Collaborator Author

jmdyck commented May 11, 2023

Added a commit to capitalize thisFooValue to ThisFooValue.

(Force-pushed because I rebased to main in the meantime.)

@michaelficarra michaelficarra added ready to merge Editors believe this PR needs no further reviews, and is ready to land. and removed ready to merge Editors believe this PR needs no further reviews, and is ready to land. labels Jul 19, 2023
@michaelficarra
Copy link
Member

Removed ready to merge since @jmdyck suggested we merge #3123 first.

@jmdyck
Copy link
Collaborator Author

jmdyck commented Jul 27, 2023

Force-pushed to resolve conflict from #3123 (re elimination of thisTimeValue). Should be okay to get "ready to merge" again.

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Jul 27, 2023
jmdyck added 2 commits August 5, 2023 10:56
Specifically:
 - thisBooleanValue
 - thisSymbolValue
 - thisNumberValue
 - thisBigIntValue
 - thisStringValue
@ljharb ljharb merged commit e519fc4 into tc39:main Aug 4, 2023
jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Aug 8, 2023
... that I inadvertently un-defined in PR tc39#3063.
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Aug 16, 2023
... that I inadvertently un-defined in PR tc39#3063.
ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Aug 16, 2023
... that I inadvertently un-defined in PR tc39#3063.
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
Specifically:
 - thisBooleanValue
 - thisSymbolValue
 - thisNumberValue
 - thisBigIntValue
 - thisStringValue
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
zhangenming pushed a commit to zhangenming/ecma262 that referenced this pull request Dec 22, 2023
... that I inadvertently un-defined in PR tc39#3063.
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.

5 participants