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

Fix #4283: error message for implicit call #4585

Merged

Conversation

helixbass
Copy link
Collaborator

Fixes #4283

@lydell is this what you had in mind? I made the error message unexpected implicit function call, and it points to the identifier that was being implicitly called (almost seems like it should point to the space between the implicit function and its first arg, but this probably clarifies the error sufficiently?)

Copy link
Collaborator

@lydell lydell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't remember exactly what I had in mind about a year ago, but this seems great!

@lydell lydell merged commit b1b34d3 into jashkenas:master Jun 26, 2017
@GeoffreyBooth
Copy link
Collaborator

Should this have been merged into master?

@lydell
Copy link
Collaborator

lydell commented Jun 26, 2017

Yes, it's harmless.

@GeoffreyBooth
Copy link
Collaborator

GeoffreyBooth commented Jun 30, 2017

@lydell @helixbass when I merge this into 2 and resolve conflicts, then run cake build:except-parser && cake test, I get:

/Users/Geoffrey/Sites/coffeescript/lib/coffeescript/coffeescript.js:44
        throw helpers.updateSyntaxError(err, code, options.filename);
        ^

Cakefile:46:33: error: unexpected end of input
  spawnNodeProcess ['bin/coffee'].concat(args), 'stderr', (status) ->
                                ^

See https://github.com/GeoffreyBooth/coffeescript/tree/merge-error-message-implicit-call (I didn’t check in lib, so that you could build the compiler).

@helixbass
Copy link
Collaborator Author

@GeoffreyBooth I merged master into 2 on my 2_merge_master branch and opened #4597. I think it was a question of how rewriter.coffee was merged. But also found that the test case from #4585 needed to be updated b/c of #4493 grammar changes. As I commented in #4597, my only question regarding the merge conflicts was whether the "CoffeeScript 2 is coming..." line in documentation/sections/introduction.md should be preserved or not, as of now I've deleted it

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

Successfully merging this pull request may close these issues.

3 participants