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: NormalCompletion/ThrowCompletion create a Completion Record #2648

Merged
merged 1 commit into from
Feb 2, 2022

Conversation

ljharb
Copy link
Member

@ljharb ljharb commented Feb 2, 2022

Currently, "return Completion" links to the Completion() AO, which isn't what's happening here.

@ljharb ljharb added editorial change completion records Relates to completion records, and ? / ! notation. labels Feb 2, 2022
@ljharb ljharb requested review from syg, michaelficarra, bakkot and a team February 2, 2022 23:01
@ljharb
Copy link
Member Author

ljharb commented Feb 2, 2022

(updated to fix the remaining instances)

@bakkot bakkot added the ready to merge Editors believe this PR needs no further reviews, and is ready to land. label Feb 2, 2022
@ljharb ljharb merged commit 15a7d8a into tc39:main Feb 2, 2022
@ljharb ljharb deleted the completion-record-link branch February 2, 2022 23:27
@jmdyck
Copy link
Collaborator

jmdyck commented Feb 3, 2022

Currently, "return Completion" links to the Completion() AO, [...]

Hm. I've got a copy of the Jan 20 version of the rendered spec in my browser, and in occurrences of Return Completion {...}, the "Completion" links to 6.2.3 The Completion Record Specification Type, not to the Completion AO in 5.2.3.1 Implicit Completion Values. (In fact, nothing links to the Completion AO, not even Completion(...) calls!)

Looking at the diff of the rendered specs, there wasn't a change to the <a> element surrounding "Completion [Record]", there was only a change in the generated <emu-xref> element. Presumably browsers pay attention to the <a> element, not the <emu-xref> element.

I'm fine with the changes of this PR, it's just the quoted sentence that seems off.

@ljharb
Copy link
Member Author

ljharb commented Feb 3, 2022

hmm, when i clicked in the deploy preview on "Completion" i could have sworn it took me to the AO, which is how I noticed the change. Either way, this change seems like an improvement :-)

(i may have noticed it in the deploy preview for #2547)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
completion records Relates to completion records, and ? / ! notation. 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