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] Allow chained access of a property of a function literal #4665

Merged

Conversation

GeoffreyBooth
Copy link
Collaborator

Implements the proposed syntax discussed in #4576:

->
  a
.call(…)

This doesn’t “fix” #4576 per se, because the output desired by that issue’s OP was simply incorrect and was fixed in 1.12.6; but this allows an alternative way to chain after function literals without needing to indent (“fixing” #4576 in the sense that the issue was about avoiding Christmas trees while chaining).

@helixbass is this what you had in mind?

@helixbass
Copy link
Collaborator

@GeoffreyBooth looks good. The description is not quite right, this will change the compilation of the example from #4576 from breaking (current behavior) or compiling to something unintuitive (pre-1.12.6 behavior) to compiling in a way that's more consistent with other chaining and with the chaining-after-top-level-function-literal example you give

@GeoffreyBooth
Copy link
Collaborator Author

Fortunately the description doesn't need to be merged. 😄 @lydell, any notes?

@lydell
Copy link
Collaborator

lydell commented Aug 28, 2017

Well, if it makes

Promise
.resolve()
.then ->
  yes
.then ok

compile with such a small change then it looks good to me.

@GeoffreyBooth
Copy link
Collaborator Author

Hmm, @lydell your example compiles now. I guess I should change the second test.

The first test is definitely good,

->
  eq @a, 3
.call a: 3

doesn’t currently compile. @helixbass is there another example you can think of that doesn’t compile now, that should after this?

@lydell
Copy link
Collaborator

lydell commented Aug 28, 2017

@GeoffreyBooth I copied my example from the tests of this PR!

@GeoffreyBooth
Copy link
Collaborator Author

I just meant, that test is bad because it compiles fine without this PR. So it’s not testing the change that this PR makes. It might be a good test to keep anyway, as a general test of chaining, but I feel like I should have at least one more test specific to this PR’s change.

@GeoffreyBooth
Copy link
Collaborator Author

Thinking about this more, it occurs to me that the only properties or methods you could access off a function literal are the ones attached to Function itself.call, .apply, .bind etc. Which is useful functionality to add, but maybe we don’t need separate tests for more than just .call.

I think I’ll merge this in, and if anyone suggests additional tests I’ll add them in a future PR.

@GeoffreyBooth GeoffreyBooth merged commit d7d69a4 into jashkenas:2 Aug 28, 2017
@GeoffreyBooth GeoffreyBooth deleted the deindented-function-chaining branch August 28, 2017 20:16
@GeoffreyBooth GeoffreyBooth mentioned this pull request Sep 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants