Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Support yield statements #19

Merged
merged 1 commit into from
Nov 3, 2015

Conversation

jamestalmage
Copy link
Contributor

From what I can tell, there is no way to get esprima to parse a yield statement that is not wrapped in a generator function.

To work around this, I do the following:

  1. Try to parse normally first
  2. Failing that, try parsing by wrapping code in a generator function. If it works, then unwrap the AST.

This will require additional modifications on the instrumentation side of things before the output looks correct (my test will need to be modified at that point). The result of the yield is not captured.

@jamestalmage jamestalmage force-pushed the jt-yield-statements branch 3 times, most recently from 954f91a to d14e0a8 Compare November 3, 2015 07:26
@jamestalmage
Copy link
Contributor Author

@twada
I'm going to bump to espower 1.1.0 in package.json. Unless you publish in the next few seconds my next push is going to fail CI. Just re-trigger via Travis web page once the release is cut and it should pass again.

Also, just FYI - I don't know if you are an Intellij user, but if so, I hope you find this useful: https://github.com/jamestalmage/intellij-code-to-string-js

'}',
'',
'var gen = myGenerator(3);',
'gen.next().value.then((val) => gen.next(val)).catch(done);'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This exhausts the generator similar to how co would.

We could also just pull in co as a dev dependency if you want, but I opted to avoid adding another one for now.

Copy link
Member

Choose a reason for hiding this comment

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

but I opted to avoid adding another one for now.

Agreed. Thanks.

@twada
Copy link
Member

twada commented Nov 3, 2015

@jamestalmage I've released espower 1.1.0 and restarted Travis build.

Also, just FYI - I don't know if you are an Intellij user, but if so, I hope you find this useful: https://github.com/jamestalmage/intellij-code-to-string-js

Wow this is so useful for creating our tests!
Yes, I'm using WebStorm (and yeah, Emacs 😜).

@jamestalmage
Copy link
Contributor Author

woot! Green!

try {
top = ast = esprima.parse(jsCode, parserOptions());
} catch (e) {
ast = esprima.parse(wrappedInGenerator(jsCode), parserOptions());
Copy link
Member

Choose a reason for hiding this comment

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

Would you add inline comments explaining why this catch clause is needed and how it work? (just like a description of this pull-req)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done!

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!!

From what I can tell, there is no way to get esprima to parse a
yield statement that is not wrapped in a generator function.
To work around this: try to parse normally first, then try
parsing by wrapping with a generator statement. If it works
the second time, then we assume it is due to a yield statement.
@twada
Copy link
Member

twada commented Nov 3, 2015

@jamestalmage Thank you for your great contribution!
I'm going to cut a new release (as 1.2.0) soon.

twada added a commit that referenced this pull request Nov 3, 2015
@twada twada merged commit 5198293 into power-assert-js:master Nov 3, 2015
@jamestalmage
Copy link
Contributor Author

awesome! Thanks!

@jamestalmage jamestalmage deleted the jt-yield-statements branch November 3, 2015 10:23
@twada
Copy link
Member

twada commented Nov 3, 2015

Just released power-assert-formatter 1.2.0. Thank you for your great contribution!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants