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

### */ ### #1050

Closed
satyr opened this issue Jan 16, 2011 · 11 comments
Closed

### */ ### #1050

satyr opened this issue Jan 16, 2011 · 11 comments

Comments

@satyr
Copy link
Collaborator

satyr commented Jan 16, 2011

$ coffee -e '### */ ###'

.:2
  /* */ */
        ^
SyntaxError: Unexpected token *
@michaelficarra
Copy link
Collaborator

Nice catch.

@TrevorBurnham
Copy link
Collaborator

But how should this be fixed? Generate a compiler error if a block comment contains */? Change each instance of */ to *//*? (What would then happen to /*/?) How can the user's intent best be preserved?

I'm guessing that if someone puts */ in a block comment, they'd like it to show up as */ in a documentation generator, e.g. Docco. Also, the * should be preserved because they might be using Markdown. So, my proposal is to replace */ with */ (where / is the HTML escape sequence for /). It's not a perfect solution, but it should cover most cases.

@michaelficarra
Copy link
Collaborator

I agree, Trevor. Using the escape sequence for an asterisk is also a solution. Any reasons to pick one over the other?

@AlyxRen
Copy link

AlyxRen commented Jan 17, 2011

the reason he suggested using the escape for '/' becaue the * may be used in Markdown evaluation.

@michaelficarra
Copy link
Collaborator

Well, that's a good enough reason for me.

Looks like we can just change line 433 of nodes.coffee to

code = '/*' + multident(@comment.replace(/\*\//g,'*/'), @tab) + '*/'

@michaelficarra
Copy link
Collaborator

Opened pull request #1053, fixing this issue.

@jashkenas
Copy link
Owner

I'm afraid that translating to HTML entities isn't an acceptable solution. This is CoffeeScript to JavaScript -- not CoffeeScript to JS-that-might-be-displayed-within-HTML. If */ is an illegal sequence within a JS block comment, and there's no way to escape the slash (being a comment, with purely visual value), then it should be an illegal sequence in CoffeeScript as well. Patched at SHA: 4b78790

@satyr
Copy link
Collaborator Author

satyr commented Jan 19, 2011

FYI, Coco went ahead and fixed the syntax to /* */.
### ### was inadequate for several reasons:

  • Interferes with line comment, complicating both syntaxes.
  • Oddball as a hereX variant (all others take the form QQ -> QQQQQQ).
  • Awkward when you want:
    ###*
  • This bug.

@michaelficarra
Copy link
Collaborator

I wouldn't be against changing coffee's multiline comments to use /* */ syntax, but I'm probably alone.

@TrevorBurnham
Copy link
Collaborator

I'd be fine with that, too. Can't think of any reason to prefer ###, really.

@ozra
Copy link

ozra commented Jan 27, 2011

Agreed. Heck - why not both!? AND I still like the proposition of converting */ to / - perhaps an optional modifier character after ### to mark the block for safe guarding?

###
Documentary stuff
* Below you'll find code for the magic things that has soon to work!
###

/*
thisFunctionWillBeBackSoon()
    doMagicThatsAlmostWorkingByNow()
*/

# Yeah! Everybody wins!


###!
commented out and safe guarded */ */ */

function whyPhpStinks($beVerbose) {
    return "It's the scopes and..." /* let's just leave it! */
}
    function someJSFunction(param /*, thisp*/) { doStuff(param) }
###

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants