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

Allow multiline comment at end of an object definition [Fixes #3761] #3802

Merged
merged 1 commit into from Feb 11, 2015

Conversation

Projects
None yet
3 participants
@mapmeld
Contributor

mapmeld commented Jan 24, 2015

#3761 appears to only be an issue when the multiline comment is indented, and it appears at the end of an object definition

includes test

Issue #3735 appears to be unaffected by this patch - that may not be caused by rewriter.coffee at all.

@mapmeld

This comment has been minimized.

Show comment
Hide comment
@mapmeld

mapmeld Jan 29, 2015

Contributor

@jashkenas any feedback on whether this would work? I could add additional tests if it seems weird to change the rewriter

Contributor

mapmeld commented Jan 29, 2015

@jashkenas any feedback on whether this would work? I could add additional tests if it seems weird to change the rewriter

@jashkenas jashkenas added the change label Jan 29, 2015

@lydell

View changes

Show outdated Hide outdated test/comments.coffee
@@ -418,3 +418,12 @@ test "#3638: Demand a whitespace after # symbol", ->
"""
eq CoffeeScript.compile(input, bare: on), result
test "#3761: Multiline comment inside object", ->

This comment has been minimized.

@lydell

lydell Feb 10, 2015

Collaborator

inside -> at the end of

would be more accurate

@lydell

lydell Feb 10, 2015

Collaborator

inside -> at the end of

would be more accurate

@lydell

View changes

Show outdated Hide outdated src/rewriter.coffee
@@ -305,7 +305,10 @@ class exports.Rewriter
# the continuation of an object.
else if inImplicitObject() and tag is 'TERMINATOR' and prevTag isnt ',' and
not (startsLine and @looksObjectish(i + 1))
endImplicitObject()
if nextTag && nextTag == 'HERECOMMENT'

This comment has been minimized.

@lydell

lydell Feb 10, 2015

Collaborator

return forward 1 if nextTag is 'HERECOMMENT'

@lydell

lydell Feb 10, 2015

Collaborator

return forward 1 if nextTag is 'HERECOMMENT'

@lydell

This comment has been minimized.

Show comment
Hide comment
@lydell

lydell Feb 10, 2015

Collaborator

Apart from my comments, LGTM. +1 for merging.

Collaborator

lydell commented Feb 10, 2015

Apart from my comments, LGTM. +1 for merging.

@jashkenas

This comment has been minimized.

Show comment
Hide comment
@jashkenas

jashkenas Feb 10, 2015

Owner

@mapmeld Want to make those tweaks?

Owner

jashkenas commented Feb 10, 2015

@mapmeld Want to make those tweaks?

@mapmeld

This comment has been minimized.

Show comment
Hide comment
@mapmeld

mapmeld Feb 11, 2015

Contributor

@lydell @jashkenas thanks for the feedback - those changes should be in the PR now

Contributor

mapmeld commented Feb 11, 2015

@lydell @jashkenas thanks for the feedback - those changes should be in the PR now

@lydell

This comment has been minimized.

Show comment
Hide comment
@lydell

lydell Feb 11, 2015

Collaborator

Please squash the commits

Collaborator

lydell commented Feb 11, 2015

Please squash the commits

jashkenas added a commit that referenced this pull request Feb 11, 2015

Merge pull request #3802 from mapmeld/multiline_comment_fix
Allow multiline comment at end of an object definition [Fixes #3761]

@jashkenas jashkenas merged commit 8130e63 into jashkenas:master Feb 11, 2015

@jashkenas jashkenas added the fixed label Feb 11, 2015

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