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

Trailing commas in function param and call lists #711

Merged
merged 1 commit into from Nov 1, 2016

Conversation

jeffmo
Copy link
Member

@jeffmo jeffmo commented Oct 13, 2016

The function trailing commas proposal
is now Stage 4, so this PR updates the spec text to reflect it.

Please review carefully as I'm fairly noobish at merging ecmaspeak.

Copy link
Member

@leobalter leobalter left a comment

Choose a reason for hiding this comment

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

LGTM. There's just a small detail missing ('.') in a comment above below.

1. Return _names_.
1. Let _names_ be BoundNames of |FormalParameterList|
1. Append to _names_ the BoundNames of |FunctionRestParameter|
1. Return _names_
Copy link
Member

Choose a reason for hiding this comment

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

missing .s

1. Let _status_ be the result of performing IteratorBindingInitialization for |FormalsList| using _iteratorRecord_ and _environment_ as the arguments.
1. ReturnIfAbrupt(_status_).
1. Let _restIndex_ be the result of performing IteratorBindingInitialization for |FormalParameterList| using _iteratorRecord_ and _environment_ as the arguments.
1. ReturnIfAbrupt(_restIndex_).
Copy link
Member

Choose a reason for hiding this comment

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

this change is not wrong but neither necessary. status would be fine.

No need to revert, but is there any specific reason for this change?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, I think this was just me copying the algo from FormalParameters : FormalParameterList,FunctionRestParameter right above (and me deleting FormalsList before realizing I needed to replace it).

Anyway, status does make more sense. Will switch it back

The [function trailing commas proposal](https://tc39.github.io/proposal-trailing-function-commas/)
is now Stage 4, so this PR updates the spec text to reflect it.

Please review carefully as I'm fairly noobish at merging ecmaspeak.
@jeffmo
Copy link
Member Author

jeffmo commented Oct 14, 2016

Updated to address @leobalter's feedback

@jeffmo
Copy link
Member Author

jeffmo commented Oct 19, 2016

cc @bterlson -- I guess you're the one who needs to review this before merging?

@bterlson
Copy link
Member

@jeffmo I'll hit the merge button once I've reviewed, don't worry. Next week once I'm back!

@bterlson
Copy link
Member

bterlson commented Nov 1, 2016

Tests are in, I've reviewed yet again, pulling this in.

@bterlson
Copy link
Member

bterlson commented Nov 1, 2016

Sorry had to reconvince myself that the grammar works :-P

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.

None yet

3 participants