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

object literal #1111

Closed
dvv opened this issue Feb 5, 2011 · 13 comments
Closed

object literal #1111

dvv opened this issue Feb 5, 2011 · 13 comments
Labels

Comments

@dvv
Copy link

dvv commented Feb 5, 2011

obj =
  foo: bar 'baz', () -> id: nonce()

PARSE ERROR ON LINE 2: UNEXPECTED ','

obj = {
  foo: bar 'baz', () -> id: nonce()
}

OK

obj =
  foo: bar('baz', () -> id: nonce())

OK too

Best regards,
--Vladimir

@satyr
Copy link
Collaborator

satyr commented Feb 6, 2011

Reducing to:

$ coffee -ne 'k: f a, () ->'
Error: Parse error on line 1: Unexpected ','
...

$ coffee -te 'k: f a, () ->'
[{ {] [IDENTIFIER k] [: :] [IDENTIFIER f] [CALL_START (]
[IDENTIFIER a] [CALL_END )] [} }] [, ,]
[PARAM_START (] [PARAM_END )] [-> ->]
[INDENT 2] [OUTDENT 2] [TERMINATOR \n]

@aeosynth
Copy link
Contributor

you can reduce it a bit further:

$ coffee -bpe 'k: f a, ->'
Error: Parse error on line 1: Unexpected ','

$ coffee -te 'k: f a, ->'
[{ {] [IDENTIFIER k] [: :] [IDENTIFIER f] [CALL_START (] [IDENTIFIER a] [CALL_END )] [} }] [, ,] [-> ->] [INDENT 2] [OUTDENT 2] [TERMINATOR \n]

@TrevorBurnham
Copy link
Collaborator

So to review and bring in #1143:

k: f a, b

is fine (yielding ({k: f(a, b)}); whence the extra parentheses?), but

k: f a, (b)

k: f a, null

k: f a, ->

k: f a, {}

k: f a, "#{b}"

all give the same bizarre error message described above. So there's a special case being made for k: f a, b, and all other cases are failing miserably, correct?

Note that all of the above work fine with explicit curly braces.

@MichaelBlume
Copy link

Honestly, it seems like a feature to me if the first line doesn't compile, because it means you'll be forced to use parens to indicate your intended structure, and I won't have to guess at the intent when I read it.

@michaelficarra
Copy link
Collaborator

@MichaelBlume: Which first line? I don't see any ambiguous syntax anywhere in this issue.

@MichaelBlume
Copy link

"Guess the intent" was a poor choice of words on my part. If I think about it for a moment, I can see that the code must mean one and only one thing. "If I think about it" is the problem here. Code should be as transparent in meaning as possible. Up to a point, excessive punctuation occludes meaning, but in this case, a well-placed pair of parens would be more readable.

@aeosynth
Copy link
Contributor

I found another case:

k: f a, []

the b is unnecessary in the first case - this errors:

k: f a, ()

edit: second case
edit: third case, b unnecessary
edit: rewrite

@michaelficarra
Copy link
Collaborator

@aeosynth: Maybe the b is unnecessary to produce the error, but it's necessary to create a snippet that is expected to work and instead produces the error.

@TrevorBurnham
Copy link
Collaborator

I've posted a bug bounty for this.

@satyr
Copy link
Collaborator

satyr commented Sep 6, 2011

Rewriter bugs are particulary tough--$10 seems rip-off.

@michaelficarra
Copy link
Collaborator

Agreed with @satyr, but it's still better than no reward and we can still have more than one person contribute a bounty.

@zmthy
Copy link
Collaborator

zmthy commented Nov 24, 2011

The problem lies in that solving this problem is left to the filterImplicitObjects method in the Call node class. This works for a b:c, d because the grammar accepts the d for DRY syntax. It shouldn't pass through at all for what the intention is. Of course, the DRY syntax doesn't accept anything more complicated, so nothing more complicated passes through. In order to fix this bug, this solution needs to be moved to the rewriter.

@jaekwon
Copy link

jaekwon commented Jan 27, 2012

Fixed on my fork. See commit ece0bc9, which relies on my rewriter.coffee rewrite, #2017.

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

9 participants