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

Fix Ex 5 in the tutorial to match its expansion #564

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

felipethome
Copy link

The expansion of the example 5 in the tutorial, with sweet.js version 2.0.0, gives for me:

(function (bb8_5) {
  ;
  console.log(bb8_5.beep());
}(new Droid("BB-8", "orange")));

So I added one more ctx.next() and rebuild the docs. Maybe would be better also a check to see if the ; exists?

@disnet
Copy link
Member

disnet commented Aug 3, 2016

Right, you would need to check to make sure the ; exists for the example to really work but that just makes the example more confusing than necessary. Better to have a harmless empty statement in the expansion output imo.

@felipethome
Copy link
Author

But don't you think it is nice for the user to see ; needs to be eaten too? Also, all the examples have their corresponding expansion, but in this particular case the expansion is misleading because it doesn't contain the ;. If you disagree feel free to close it @disnet, and thanks for the attention.

@disnet
Copy link
Member

disnet commented Aug 4, 2016

Actually, why don't you remove the ; from the source? That demonstrates that expansion is ASI aware but avoids complicating the macro code.

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.

2 participants