Skip to content

[WIP][CS2] Block comments in array literals #4541

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6076b51
Comments tests should use same `toJS` function as the other tests
GeoffreyBooth May 6, 2017
36c8d0e
Add simple multiline comment in array test
GeoffreyBooth May 7, 2017
8821642
Array grammar should mirror object grammar
GeoffreyBooth May 7, 2017
89ab2ed
Multiline/block comments are now expressions, not statements, giving …
GeoffreyBooth May 7, 2017
956cdbf
Block comments are no longer ignored in the ‘directive prologue’, so …
GeoffreyBooth May 7, 2017
c1b3b80
Block comments are no longer statements, so an empty conditional no l…
GeoffreyBooth May 7, 2017
fc06f2e
Update tests for new output that lacks a preceding newline; add tests…
GeoffreyBooth May 7, 2017
1569dd4
We should keep this test
GeoffreyBooth May 7, 2017
e8286ba
Now that block comments are expressions, `if` expressions need to out…
GeoffreyBooth May 7, 2017
36f665b
Block comment in an array shouldn’t be its own element
GeoffreyBooth May 7, 2017
2137f72
Style/comment cleanup
GeoffreyBooth May 8, 2017
abd33d7
Make block comments statements again; make sure they don’t get wrappe…
GeoffreyBooth May 8, 2017
cb750a6
Add test for invalid JS
GeoffreyBooth May 9, 2017
d05a168
Reset output
GeoffreyBooth May 9, 2017
4fb39f7
Merge branch '2' of github.com:jashkenas/coffeescript into comment-bl…
GeoffreyBooth May 14, 2017
029d71a
Throw an error on comments that would be wrapped in closures, as they…
GeoffreyBooth May 14, 2017
be549a6
Merge branch '2' into comment-block-in-array-literal
GeoffreyBooth May 14, 2017
df5e27c
Merge branch '2' into comment-block-in-array-literal
GeoffreyBooth May 18, 2017
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions lib/coffeescript/grammar.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

26 changes: 19 additions & 7 deletions lib/coffeescript/nodes.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

173 changes: 85 additions & 88 deletions lib/coffeescript/parser.js

Large diffs are not rendered by default.

16 changes: 12 additions & 4 deletions src/grammar.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -464,8 +464,7 @@ grammar =

# The array literal.
Array: [
o '[ ]', -> new Arr []
o '[ ArgList OptComma ]', -> new Arr $2
o '[ ElementList OptComma ]', -> new Arr $2
]

# Inclusive and exclusive range dots.
Expand All @@ -487,8 +486,17 @@ grammar =
o 'RangeDots', -> new Range null, null, $1
]

# The **ArgList** is both the list of objects passed into a function call,
# as well as the contents of an array literal
# The **ElementList** holds the contents of an array literal
# (i.e. comma-separated expressions). Newlines work as well.
ElementList: [
o '', -> []
o 'Arg', -> [$1]
o 'ElementList , Arg', -> $1.concat $3
o 'ElementList OptComma TERMINATOR Arg', -> $1.concat $4
o 'ElementList OptComma INDENT ElementList OptComma OUTDENT', -> $1.concat $4
]

# The **ArgList** is the list of objects passed into a function call,
# (i.e. comma-separated expressions). Newlines work as well.
ArgList: [
o 'Arg', -> [$1]
Expand Down
19 changes: 13 additions & 6 deletions src/nodes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,10 @@ exports.Base = class Base
node = @unfoldSoak(o) or this
node.tab = o.indent
if o.level is LEVEL_TOP or not node.isStatement(o)
node.compileNode o
if node instanceof Comment and not o.level is LEVEL_TOP
node.error 'block comments not permitted here'
else
node.compileNode o
else
node.compileClosure o

Expand Down Expand Up @@ -765,7 +768,7 @@ exports.Comment = class Comment extends Base
comment = @comment.replace /^(\s*)#(?=\s)/gm, "$1 *"
code = "/*#{multident comment, @tab}#{if '\n' in comment then "\n#{@tab}" else ''} */"
code = o.indent + code if (level or o.level) is LEVEL_TOP
[@makeCode("\n"), @makeCode(code)]
[@makeCode(code)]

#### Call

Expand Down Expand Up @@ -1223,10 +1226,12 @@ exports.Arr = class Arr extends Base

compiledObjs = (obj.compileToFragments o, LEVEL_LIST for obj in @objects)
for fragments, index in compiledObjs
if index
answer.push @makeCode ", "
answer.push fragments...
if fragmentsToText(answer).indexOf('\n') >= 0
if fragments[0].type is 'Comment'
answer.push @makeCode ' '
else unless index is compiledObjs.length - 1
answer.push @makeCode ', '
if fragmentsToText(answer).includes('\n')
answer.unshift @makeCode "[\n#{o.indent}"
answer.push @makeCode "\n#{@tab}]"
else
Expand Down Expand Up @@ -3086,7 +3091,9 @@ exports.If = class If extends Base
cond = @condition.compileToFragments o, LEVEL_COND
body = @bodyNode().compileToFragments o, LEVEL_LIST
alt = if @elseBodyNode() then @elseBodyNode().compileToFragments(o, LEVEL_LIST) else [@makeCode('void 0')]
fragments = cond.concat @makeCode(" ? "), body, @makeCode(" : "), alt
body.push @makeCode('void 0') if @bodyNode() instanceof Comment
alt.push @makeCode('void 0') if @elseBodyNode() instanceof Comment
fragments = cond.concat @makeCode(' ? '), body, @makeCode(' : '), alt
if o.level >= LEVEL_COND then @wrapInParentheses fragments else fragments

unfoldSoak: ->
Expand Down
34 changes: 2 additions & 32 deletions test/classes.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,6 @@ test "#1966: external constructors should produce their return value", ->
ok (new A) not instanceof A

test "#1980: regression with an inherited class with static function members", ->

class A

class B extends A
Expand All @@ -572,28 +571,6 @@ test "#1534: class then 'use strict'", ->
doesNotThrow -> CoffeeScript.run "class then #{error}", bare: yes
doesNotThrow -> CoffeeScript.run "class then #{error};'use strict'", bare: yes

# comments are ignored in the Directive Prologue
comments = ["""
class
### comment ###
'use strict'
#{error}""",
"""
class
### comment 1 ###
### comment 2 ###
'use strict'
#{error}""",
"""
class
### comment 1 ###
### comment 2 ###
'use strict'
#{error}
### comment 3 ###"""
]
throws (-> CoffeeScript.run comment, bare: yes) for comment in comments

# [ES5 §14.1](http://es5.github.com/#x14.1) allows for other directives
directives = ["""
class
Expand All @@ -607,16 +584,9 @@ test "#1534: class then 'use strict'", ->
#{error}""",
"""
class
### comment 1 ###
'directive 1'
'use strict'
#{error}""",
"""
class
### comment 1 ###
'directive 1'
### comment 2 ###
'use strict'
### comment ###
#{error}"""
]
throws (-> CoffeeScript.run directive, bare: yes) for directive in directives
Expand All @@ -632,8 +602,8 @@ test "#2052: classes should work in strict mode", ->
test "directives in class with extends ", ->
strictTest = """
class extends Object
### comment ###
'use strict'
### comment ###
do -> eq this, undefined
"""
CoffeeScript.run strictTest, bare: yes
Expand Down
6 changes: 3 additions & 3 deletions test/comments.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,6 @@ test "#3132: Format indented block-comment nicely", ->
var fn;

fn = function() {

/*
* Indented
Multiline
Expand Down Expand Up @@ -362,7 +361,6 @@ test "#3132: Place block-comments nicely", ->

DummyClass = (function() {
class DummyClass {

/**
* @constructor
*/
Expand All @@ -371,7 +369,6 @@ test "#3132: Place block-comments nicely", ->

};


/**
* Singleton reference
*
Expand Down Expand Up @@ -412,3 +409,6 @@ test "#3761: Multiline comment at end of an object", ->
test "#4375: UTF-8 characters in comments", ->
# 智に働けば角が立つ、情に掉させば流される。
ok yes

test "#3735: Multiline comment not treated as value", ->
throws -> CoffeeScript.compile 'a + ### Comment ###'