Skip to content

Conversation

rkirsling
Copy link
Member

@rkirsling rkirsling commented Jun 16, 2019

@devsnek pointed out that 14.6.13 step 10 has extremely odd spacing:

constructor(... args){ super (...args);}

Like Gus, I'd rather not battle over style here, but I think anyone can agree that that line needs fixing.

The first commit here fixes parentheses and braces to align with the style shown in 13.7.5.15's note. (Strictly speaking, this is more than necessary; even just removing spaces would be an improvement.)

The second commit normalizes a couple of ..., cases to our more typical … ,.

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.

Matching the most popular community style seems like an automatic win to me; plus; the current styling doesn’t match any version of community style that has ever existed.

@ljharb ljharb requested review from a team and zenparsing June 16, 2019 03:52
@devsnek
Copy link
Member

devsnek commented Jun 16, 2019

on a side note, is there any reason that the default constructors are defined in terms of source text instead of algorithm steps?

 - Add missing … escapes.
@ljharb ljharb force-pushed the fix-weird-code-spacing branch from cfdb7ad to 2fa96c2 Compare June 16, 2019 04:53
@ljharb
Copy link
Member

ljharb commented Jun 16, 2019

@devsnek ¯\_(ツ)_/¯

@ljharb
Copy link
Member

ljharb commented Jun 16, 2019

(per editor's group discussion; some small editorial PRs can be merged without both editors signing off)

@ljharb ljharb merged commit 2fa96c2 into tc39:master Jun 16, 2019
@rkirsling rkirsling deleted the fix-weird-code-spacing branch June 16, 2019 14:40
@jmdyck
Copy link
Collaborator

jmdyck commented Jun 16, 2019

on a side note, is there any reason that the default constructors are defined in terms of source text instead of algorithm steps?

The spec doesn't define notation for building parse nodes. (And even if it did, the notation probably wouldn't be as compact and clear as defining by source text, at least in this case.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants