Skip to content

Lexer: Chaining: Method chaining wraps enclosing object incorrectly #4533

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

Closed
xixixao opened this issue May 1, 2017 · 4 comments · Fixed by #4534
Closed

Lexer: Chaining: Method chaining wraps enclosing object incorrectly #4533

xixixao opened this issue May 1, 2017 · 4 comments · Fixed by #4534
Labels

Comments

@xixixao
Copy link
Contributor

xixixao commented May 1, 2017

Input:

x =
  a: f 3
  b: f 4
    .something()

Output:

x = {
  a: f(3),
  b: f(4)
}.something();

But consider that

x =
  a: 3 # <- no function call here
  b: f 4
    .something()

compiles to

x = {
  a: 3,
  b: f(4).something()
};

I have a fix coming.

@jashkenas
Copy link
Owner

This illustrates yet another reason why allowing chains to close implicit calls is hinkey and maybe shouldn't have been added in the first place.

You want

x =
  a: 3 # <- no function call here
  b: f one
    .something()

To be:

b: f(one).something()

But this:

x =
  a: 3 # <- no function call here
  b: f one[2].three().four()
    .five()

Looks like it really ought to be:

 b: f(one[2].three().four().five())

@vendethiel
Copy link
Collaborator

I don't think that's any more ambiguous than f -> b c, d # d is for b, not f.

@GeoffreyBooth
Copy link
Collaborator

Well, we could take the feature out entirely in 2. It was never properly documented in 1, so it wouldn’t be the hugest breaking change, but it would certainly affect a lot of people.

Or we could leave it in as it is now, a basically undocumented feature. I could close all the remaining chaining issues with the excuse that “this is a deprecated feature. It remains available in order to not break backward compatibility, but we don’t plan to make any further improvements to it” or similar.

Or we could just try to fix it as best we can. Now is the time, so that any breaking changes that result happen before 2 leaves beta.

@connec
Copy link
Collaborator

connec commented May 2, 2017

We're probably missing a definition of a 'canonical style' for writing CoffeeScript, then it would be clearer in these discussions which direction is better. In particular, some canonical definition of which whitespace constructs are supported would go a long way to explaining why some things compile the way they do and reduce the number of cases of people being 'surprised' by compilations, particularly when they were deliberately implemented (such as this one) rather than emergent from the whitespace rules.

Looks like it really ought to be:

In this case, it's because an indented . access will close any preceding implicit calls, which isn't too difficult to explain or understand. If this was well documented in the introduction to CS, as part of the syntax, then it should be easy enough to work with.

@GeoffreyBooth GeoffreyBooth changed the title Method chaining wraps enclosing object incorrectly Lexer: Chaining: Method chaining wraps enclosing object incorrectly May 6, 2017
xixixao added a commit to xixixao/coffee-script that referenced this issue May 7, 2017
xixixao added a commit to xixixao/coffee-script that referenced this issue May 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants