Inconsistent margin on ObjectLiteral evaluation semantics

[loganfsmyth]

In https://tc39.github.io/ecma262/#sec-object-initializer-runtime-semantics-evaluation, the second runtime evaluation rule for ObjectLiteral is indented too much, which interrupts the alternating style of grammar rule followed by indented evaluation steps.

Looks like the others have the collapsed attribute, but it also looks questionable collapsed onto one line, so not sure what the right fix would be.

[jmdyck]

(Note that the behavior is not particular to that one case. It looks like every single-line production in the spec is flush-left, and every multi-line production is indented.)

[jmdyck]

(Note that the behavior is not particular to that one case. It looks like every single-line production in the spec is flush-left, and every multi-line production is indented.)

[loganfsmyth]

Totally, this is just a case where some things are single-line and some aren't in the same block, which makes it look inconsistent.

[loganfsmyth]

Totally, this is just a case where some things are single-line and some aren't in the same block, which makes it look inconsistent.

[tjcrowder]

This problem can be found in ES2015 and ES2016 as well as ES2017. It significantly impairs the readability of the spec, which isn't exactly a marvel of clarity in the first place. I know I want my time back trying to figure out what the heck it meant, viz https://esdiscuss.org/topic/reading-tail-calls-specification. :-)

With respect, I suggest this is an urgent issue that needs urgent resolution.

[tjcrowder]

This problem can be found in ES2015 and ES2016 as well as ES2017. It significantly impairs the readability of the spec, which isn't exactly a marvel of clarity in the first place. I know I want my time back trying to figure out what the heck it meant, viz https://esdiscuss.org/topic/reading-tail-calls-specification. :-)

With respect, I suggest this is an urgent issue that needs urgent resolution.

[tjcrowder]

I'll try to carve out some time to take a stab at this this weekend. Seems like the kind of thing someone not deeply involved in the spec can do, saving the heavy lifters for more heavy lifting.

[tjcrowder]

I'll try to carve out some time to take a stab at this this weekend. Seems like the kind of thing someone not deeply involved in the spec can do, saving the heavy lifters for more heavy lifting.

[tjcrowder]

@bterlson -

Flagging you as this may require changes to ecmarkup (or grammarkdown, but I think it would be ecmarkup). Or it may not; you may have a better idea — in any case, would benefit from your input. :-)

I'm brand-new to this stuff, but I believe the issue (No. 1 below; "Current") is because some of the productions are collapsed in the spec's source text and some aren't. Here's a sample of the Grammarmarkdown:

FunctionStatementList : [empty]

StatementListItem : Declaration

Statement :
  VariableStatement
  EmptyStatement
  ExpressionStatement
  ContinueStatement
  BreakStatement
  ThrowStatement
  DebuggerStatement

Block : `{` `}`

Consequently we end up with some emu-production elements with collapsed and some without. The collapsed attribute on emu-production does two things to the styling: Makes the RHS(s) inline, and removes the margins. When there's a list of productions where some are collapsed and some aren't, the jagged indentation and top/bottom offsets cause confusion.

We could solve it without changing ecmarkup by adding collapsed to the emu-grammar for this part of the spec (No. 2 below; "Collapsed"), but it ends up still being fairly difficult to read.

Another option would be to not make any of those collapsed, but that too ends up not being very easy to read and being vertically verbose (No. 3 below; "Not collapsed").

Ideally (in my view), we'd be able to eliminate the margins on un-collapsed productions without making the RHSs inline (No. 4 below; "Aspire" [handcrafted]).

I don't think there's currently a way to do that in ecmarkup, is there? Should we add something? Or is there a better solution than any of the above?

In terms of an ecmarkup solution, it seems like it could be quite simple: Add a mixed-list (or whatever) attribute to emu-grammar:

<emu-grammar mixed-list>
  FunctionStatementList : [empty]

  StatementListItem : Declaration

  Statement :
    VariableStatement
    EmptyStatement
    ...
</emu-grammar>

...and style it like this:

emu-grammar[mixed-list] emu-production {
    margin: 0;
}

My tests tell me that ecmarkup is already happy to carry that attribute into the final result, so this ends up being a documentation and CSS change (which I'm happy to do and submit to ecmarkup as a pull-request).

But again, you might have a better idea? Thanks in advance.

No. 1: Current

No. 2: Collapsed

No. 3: Not collapsed

No. 4: Aspire

[tjcrowder]

@bterlson -

Flagging you as this may require changes to ecmarkup (or grammarkdown, but I think it would be ecmarkup). Or it may not; you may have a better idea — in any case, would benefit from your input. :-)

I'm brand-new to this stuff, but I believe the issue (No. 1 below; "Current") is because some of the productions are collapsed in the spec's source text and some aren't. Here's a sample of the Grammarmarkdown:

FunctionStatementList : [empty]

StatementListItem : Declaration

Statement :
  VariableStatement
  EmptyStatement
  ExpressionStatement
  ContinueStatement
  BreakStatement
  ThrowStatement
  DebuggerStatement

Block : `{` `}`

Consequently we end up with some emu-production elements with collapsed and some without. The collapsed attribute on emu-production does two things to the styling: Makes the RHS(s) inline, and removes the margins. When there's a list of productions where some are collapsed and some aren't, the jagged indentation and top/bottom offsets cause confusion.

We could solve it without changing ecmarkup by adding collapsed to the emu-grammar for this part of the spec (No. 2 below; "Collapsed"), but it ends up still being fairly difficult to read.

Another option would be to not make any of those collapsed, but that too ends up not being very easy to read and being vertically verbose (No. 3 below; "Not collapsed").

Ideally (in my view), we'd be able to eliminate the margins on un-collapsed productions without making the RHSs inline (No. 4 below; "Aspire" [handcrafted]).

I don't think there's currently a way to do that in ecmarkup, is there? Should we add something? Or is there a better solution than any of the above?

In terms of an ecmarkup solution, it seems like it could be quite simple: Add a mixed-list (or whatever) attribute to emu-grammar:

<emu-grammar mixed-list>
  FunctionStatementList : [empty]

  StatementListItem : Declaration

  Statement :
    VariableStatement
    EmptyStatement
    ...
</emu-grammar>

...and style it like this:

emu-grammar[mixed-list] emu-production {
    margin: 0;
}

My tests tell me that ecmarkup is already happy to carry that attribute into the final result, so this ends up being a documentation and CSS change (which I'm happy to do and submit to ecmarkup as a pull-request).

But again, you might have a better idea? Thanks in advance.

No. 1: Current

No. 2: Collapsed

No. 3: Not collapsed

No. 4: Aspire

[jmdyck]

I think if we just changed the margin-left for emu-production from 5ex to 0, that'd be a step in the right direction.

My guess is that the indent (and the top and bottom margins) are there just to make 'Syntax' sections nicer. If we add an attribute to definitional <emu-grammar>s (see issue #960), that would allow emu-production to have different styling in 'Syntax' sections.

[jmdyck]

I think if we just changed the margin-left for emu-production from 5ex to 0, that'd be a step in the right direction.

My guess is that the indent (and the top and bottom margins) are there just to make 'Syntax' sections nicer. If we add an attribute to definitional <emu-grammar>s (see issue #960), that would allow emu-production to have different styling in 'Syntax' sections.

[tjcrowder]

@jmdyck -

I think if we just changed the margin-left for emu-production from 5ex to 0, that'd be a step in the right direction.

Not just the left margin, the top and bottom too.

Yes, if they're just there for when example would be used in that case, then only having the margins on example would solve this problem.

[tjcrowder]

@jmdyck -

I think if we just changed the margin-left for emu-production from 5ex to 0, that'd be a step in the right direction.

Not just the left margin, the top and bottom too.

Yes, if they're just there for when example would be used in that case, then only having the margins on example would solve this problem.

[tjcrowder]

@jmdyck @bterlson Or if the indentation is really there just to serve Syntax sections, put the body of Syntax sections in a section:

<!-- es6num="7.1.3.1" -->
<emu-clause id="sec-tonumber-applied-to-the-string-type">
  <h1>ToNumber Applied to the String Type</h1>
  <p>...</p>
  <section class="syntax">
    <h2>Syntax</h2>
    <emu-grammar>
      StringNumericLiteral :::
        StrWhiteSpace?
        StrWhiteSpace? StrNumericLiteral StrWhiteSpace?
  
      ....
  </section>
  ...

...with:

.syntax emu-production {
    /*...the current margins...*/
}

...and remove the left, top, and bottom margins from emu-production.

[tjcrowder]

@jmdyck @bterlson Or if the indentation is really there just to serve Syntax sections, put the body of Syntax sections in a section:

<!-- es6num="7.1.3.1" -->
<emu-clause id="sec-tonumber-applied-to-the-string-type">
  <h1>ToNumber Applied to the String Type</h1>
  <p>...</p>
  <section class="syntax">
    <h2>Syntax</h2>
    <emu-grammar>
      StringNumericLiteral :::
        StrWhiteSpace?
        StrWhiteSpace? StrNumericLiteral StrWhiteSpace?
  
      ....
  </section>
  ...

...with:

.syntax emu-production {
    /*...the current margins...*/
}

...and remove the left, top, and bottom margins from emu-production.

[tjcrowder]

@allenwb Would putting the syntax sections in section be an issue that you can see? I can take this on next weekend if you and @bterlson are good with the solution above.

[tjcrowder]

@allenwb Would putting the syntax sections in section be an issue that you can see? I can take this on next weekend if you and @bterlson are good with the solution above.

[tjcrowder]

@allenwb @bterlson I'd like to progress this to fix the unreadability of various sections, tail-calls in particular. Could we please have your feedback on the above?

[tjcrowder]

@allenwb @bterlson I'd like to progress this to fix the unreadability of various sections, tail-calls in particular. Could we please have your feedback on the above?

[bterlson]

Seems like #960 will solve this nicely (with some added css).

[bterlson]

Seems like #960 will solve this nicely (with some added css).

[tjcrowder]

@bterlson Sounds good to me. Is the use="example" stable enough I can add it to the examples in the spec text and update the CSS for PRs? CC @rbuckton.

[tjcrowder]

@bterlson Sounds good to me. Is the use="example" stable enough I can add it to the examples in the spec text and update the CSS for PRs? CC @rbuckton.

[tjcrowder]

@bterlson - Any reason I can't progress this adding type="example" and adjusting the CSS?

[tjcrowder]

@bterlson - Any reason I can't progress this adding type="example" and adjusting the CSS?

[jmdyck]

Add type="example" to what? It's already been added to all the <emu-grammar> elements that need it in spec.html.

[jmdyck]

Add type="example" to what? It's already been added to all the <emu-grammar> elements that need it in spec.html.

[tjcrowder]

@jmdyck - Thanks. I did build the latest and saw the indentation is still off, my bad not checking for type="example" in the HTML. So it's just the CSS change to ecmarkup.css (e.g., css/elements.css in the ecmarkup repo) that needs doing then. I'll do an issue and PR for ecmarkup to do that (tiny change, but needs a lot of verification).

[tjcrowder]

@jmdyck - Thanks. I did build the latest and saw the indentation is still off, my bad not checking for type="example" in the HTML. So it's just the CSS change to ecmarkup.css (e.g., css/elements.css in the ecmarkup repo) that needs doing then. I'll do an issue and PR for ecmarkup to do that (tiny change, but needs a lot of verification).

[bterlson]

@tjcrowder sorry I meant to get to this this week but failed. Yes, should be a tiny change now I hope.

[bterlson]

@tjcrowder sorry I meant to get to this this week but failed. Yes, should be a tiny change now I hope.