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

Lexer: Chaining: Leading dot not working with do #3736

Closed
darky opened this issue Nov 22, 2014 · 11 comments
Closed

Lexer: Chaining: Leading dot not working with do #3736

darky opened this issue Nov 22, 2014 · 11 comments
Labels

Comments

@darky
Copy link

darky commented Nov 22, 2014

do ->
  "foo qwe"
.replace "qwe", "bar"
@darky
Copy link
Author

darky commented Nov 22, 2014

@xixixao Can you say something about this?

@darky darky closed this as completed Nov 22, 2014
@darky darky reopened this Nov 22, 2014
@darky
Copy link
Author

darky commented Nov 22, 2014

Temporary may use lodash/underscore _.identity

_.identity do ->
  "foo qwe"
.replace "qwe", "bar"

@xixixao
Copy link
Contributor

xixixao commented Nov 22, 2014

@Darrrk This was probably never supported. do is not a function call, I didn't consider this behavior and hence it hasn't been implemented. More readable than _.identity

(do ->
  "foo qwe"
).replace "qwe", "bar"

PR welcome.

@michaelficarra
Copy link
Collaborator

Or if you really don't want to parenthesise the multi-line function,

((x)->x()) ()->
  "foo qwe"
.replace "qwe", "bar"

@alexispurslane
Copy link

Seriously, if this isn't going to be fixed, and there is an easy, clear, and obvious way out (the parens around the do) then why is this issue even open?

@xixixao
Copy link
Contributor

xixixao commented Nov 22, 2014

@michaelficarra You are just inlining the library call, which is arguably worse than the original. @ChristopherDumas It would presumably be nice to fix this if it's not too difficult. It's open because none of the moderators are obliged (or paid) to close or comment on an issue within some fixed amount of time. Let's help them by not adding spurious comments.

@darky
Copy link
Author

darky commented Nov 24, 2014

CoffeeScript beautiful, because generally I can ignore parens.
Otherwise, why programming in CoffeeScript, if exists JavaScript with parens.
do -> - it's function call, really:

(function() {
  return "foo qwe";
})();

Therefore, it's chainable too.

@GeoffreyBooth GeoffreyBooth changed the title Chaining function not working with do Lexer: Chaining: Leading dot not working with do May 6, 2017
@GeoffreyBooth
Copy link
Collaborator

This was fixed, in the sense that it compiles now, via #4665; but the new output isn’t what you want:

do ->
  "foo qwe"
.replace "qwe", "bar"

(function() {
  return "foo qwe";
}).replace("qwe", "bar")();

Note that the (), thanks to the do, is at the end, rather than before .replace. Apparently this has always been the way that do works with chaining:

do foo
.bar
# foo.bar()

I’m not sure if anyone uses do with chained functions like this, but if we were to change it so that do only applied to the immediate next function, that would be a breaking change. But the current behavior makes sense, in that the chained version is supposed to be treated the same way as the unchained version:

do foo.bar
# foo.bar()

So having do foo\n.bar become foo().bar would be inconsistent. I’m afraid you’ll just need to use parentheses to produce what you’re intending:

(do ->
  "foo qwe"
).replace "qwe", "bar"

@helixbass
Copy link
Collaborator

@GeoffreyBooth I think the current compilation (as of #4665) is quite unintuitive/unexpected. And this chained do <IIFE> construct is one I'd like to be able to use. So I opened #4666, see there for more explanation but basically I think the inconsistency it would introduce between chained do <IIFE> vs chained do <non-IIFE> is overshadowed by the (now-dangerous since it no longer fails to compile) unexpected compilation of the useful chained do <IIFE> construct

@GeoffreyBooth
Copy link
Collaborator

Yeah I was reconsidering this today, I think you're right.

GeoffreyBooth pushed a commit that referenced this issue Aug 29, 2017
* add parens to chained do iife [Fixes #3736]

* remove debug code

* fixes from code review
GeoffreyBooth pushed a commit that referenced this issue Aug 31, 2017
* add parens to chained do iife [Fixes #3736]

* remove debug code

* fixes from code review

* handle iife with params

* add test w/ destructured param from code review
@GeoffreyBooth
Copy link
Collaborator

GeoffreyBooth commented Sep 1, 2017

Fixed via #4666 and #4672.

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

No branches or pull requests

6 participants