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

#1234 Protoype Access in :: Operator #1590

Merged
merged 4 commits into from
Aug 12, 2011

Conversation

geraldalewis
Copy link
Contributor

Issue #1234: Applying a splat to a prototype using :: applies the splat to the wrong object
Opened by: @Dykam
Date opened: March 25, 2011
Milestone: 1.2

Thanks to @michaelficarra for pointing to a solution and review!

Dykam's illustrative code:

A.prototype.method array... # Compiles correctly
A::method array... # Compiles incorrectly

var _ref;
(_ref = A.prototype).method.apply(_ref, array); // Correct
A.prototype.method.apply(A, array); // Incorrect

Solution: Adding an intermediary prototype Access node between the class and the method within the grammar.

I have some questions regarding some possible refinements posted in the src/ commit.

This issue ties into a larger issue on how :: is handled ( with respect to @ ) -- I will open a new issue that addresses the super-issue within two days.

@geraldalewis
Copy link
Contributor Author

Here's a full diff of the entire patch (since the diff in Commits <> only shows the changes between the last two commits).

@Dykam
Copy link

Dykam commented Aug 12, 2011

Nice to see progress :] I wonder, why do you gist a diff and not commit to a fork?

@geraldalewis
Copy link
Contributor Author

@Dykam I will if that makes life easier for the collabs or owner; just didn't seem like a big enough change to warrant another branch?

@michaelficarra
Copy link
Collaborator

I see all 3 commits and a combined diff. No idea what you guys are talking about.

@Dykam
Copy link

Dykam commented Aug 12, 2011

The diff didn't get on the mail, so I looked over it, never mind.

class A
b: (arr) ->
eq this.toString(), nonce
toString: -> nonce
Copy link
Collaborator

Choose a reason for hiding this comment

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

To be a little less confusing:

class C
  method: -> @nonce
  nonce: nonce

arr = []
eq nonce, C::method arr... # should be applied to `C::`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes -- that looks better. I'll update in a moment :)

@michaelficarra
Copy link
Collaborator

Awesome patch, @geraldalewis. LGTM.

@geraldalewis
Copy link
Contributor Author

Thanks to you @michaelficarra :)

@jashkenas
Copy link
Owner

Gorgeous patch.

jashkenas added a commit that referenced this pull request Aug 12, 2011
@jashkenas jashkenas merged commit 42f2bd9 into jashkenas:master Aug 12, 2011
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