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

[CS2] Refine #4666: add parens to chained do IIFE with params #4672

Merged

Conversation

helixbass
Copy link
Collaborator

@connec pointed out that #4666 didn't handle do <IIFE> with arguments

So this extends #4666 to handle arguments. Added tests where the arguments are on separate lines and contain nested PARAM_END token

@jashkenas
Copy link
Owner

jashkenas commented Aug 30, 2017

And if the arguments have default values or are destructuring?

(Let's revert the previous, and do this in the grammar — instead of continuing to tweak this rule.)

@helixbass
Copy link
Collaborator Author

@jashkenas added a test including destructured param (it was already working) - it's using the rewriter's detectEnd(), which takes nesting into account, so I don't think there will be weird specific types of params that this won't handle

My only argument for keeping it this way for now is that the pre-#4666 behavior shouldn't go into 2.0.0. It's obvious that a grammar-based solution will be superior but unless someone wants to get it done pre-2.0.0 (which seems impending?), I think this is ok as a stopgap. I was tempted to go ahead and try and tackle it myself now but don't think I'm quite there yet

@jashkenas
Copy link
Owner

Cool!

@GeoffreyBooth
Copy link
Collaborator

@connec this look good to you?

@helixbass I would say go ahead and try putting this into the grammar as a follow-up PR. Even if it slips into 2.0.1, that would be fine. Just because we release 2.0.0 doesn't mean development will stop.

Copy link
Collaborator

@connec connec left a comment

Choose a reason for hiding this comment

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

My lexer-foo is still not great, but it passes the most convoluted test I can think of so 👍

do (a = (b) ->) ->
  a
.c
((function(a) {
  return a;
})(function(b) {})).c;

@GeoffreyBooth
Copy link
Collaborator

If you wanted to pass arguments into an IIFE, wouldn't you need to not use do?

((a) ->
  {b: a}
)(1).b

@helixbass
Copy link
Collaborator Author

@GeoffreyBooth you can do it via default-valued args to do, at least:

do (a=1) ->
  {b: a}
.b

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.

4 participants