using 'yield' automatically turns functions into generators

[alubbe]

See discussion #3078

[alubbe]

I have not included tests on purpose. Generators not yet widely supported so it would break tests for too many people.

At the same time, the new functionality is clean, simple and allows node.js developers like me to code very fast and memory-efficient callback solutions (see bluebird)

[alubbe]

Just to illustrate the point of why this is awesome, using bluebird and this pull request you can write the below code. Super simple to read, and supports the tried & true try, catch, finally for error handling.

delay = (ms) ->
  new Promise (f) ->
    setTimeout f, ms

Promise.spawn ->
  yield delay 500
  console.log "Well, "
  yield delay 500
  console.log "isn't this"
  yield delay 500
  console.log "awesome!"
.then(morePromises)

[jashkenas]

@almost — What do you think about how this PR does things?

[almost]

Looks nice, the improvements to the CoffeeScript code recently allow a nice and neat implementation :)

I think yieldfrom is pretty useful to have too, but this is the main thing.

[almost]

@alubbe I'm not sure I get that example, you're yielding the result of delay called with a single argument (500) so func in delay is going to be undefined, right?

[alubbe]

My bad, wrong piece of code. The function I meant is

delay = (ms) ->
  new Promise (f) ->
    setTimeout f, ms

[almost]

Oh cool, that makes sense!

[alubbe]

@jashkenas
What's your take on this PR? It's more lightweight than the other one, doesn't break anything and really helps out node.js devs - it would be great to get it in.

[jashkenas]

I dig it. We should probably find a good way to introduce it.

[alubbe]

I agree. What do you have in mind/where can I help?

[xixixao]

You could add compilation tests?

[alubbe]

So I was originally opposed to unit tests because they would break the existing test suite for everyone who is not using node 0.11 and the --harmony or --harmony-generators tags. I have come up with a solution where the generators test checks each passed argument in process.execArgv and only executes if it finds harmony somewhere in there. To actually execute it, you can start pass the harmony argument to the test suite by using npm run-script test-harmony.

[xixixao]

# ensure that these tests are only run if generators are available
generatorsAreAvailable = ->
  for execArg in process.execArgv when execArg is '--harmony'
    return yes
  no

return unless generatorsAreAvailable()

[michaelficarra]

No, please do not use top level returns. While node accepts it, it is invalid JavaScript and static analysis tools will reject it.

[xixixao]

My bad, I was mostly concerned with the 1 and 0 :)

[alubbe]

Okay, I cleaned up the test suite so that the generator test file does not even get loaded when generators are unavailable. This avoids any kind of runtime errors that the function*() notation might cause.

Let me know what you guys think.

[xixixao]

Honestly I'd prefer the previous version, now if I go and change the file and don't see an error it is not obvious why it is not getting run, I'd keep it in that file until we can remove it. No harm in having that if statement. I'd also try to avoid somethingIsnt = false.

[alubbe]

Actually, nodejs will pre-load the test, even beyond the if statement, therefore throwing an error on function*(). The only way to avoid is to not load the test file.
I think the chances of breaking the generator tests while working on other functionality are pretty slim, since the amount of new code is quite small. And anybody who actually modifies the generator behaviour is using the --generator flag and will thus run the right tests.

I'll modify the code to avoid double-negatives.

[epidemian]

Another opinion on the readability of this bit of code: wouldn't this be the same as:

generatorsAvailable = '--harmony' in process.execArgv or 
  '--harmony-generaors' in process.execArgv
# Or...
generatorsAvailable = process.execArgv.some (arg) -> 
  arg in ['--harmony', '--harmony-generators']

?

I find both of them more straightforward to read. But, i usually prefer single assignment variables if possible, so it probably is a very biased opinion =P

[xixixao]

And I'd expect break after this line. Is execArgv.some guaranteed to be defined?

[epidemian]

Is execArgv.some guaranteed to be defined?

IDK if the compiler is supposed to run on old IE (if it is: does someone test that?), but, in case it is, there seems to be a helper for that available.

[vendethiel]

I guess it should work on oldIE, yes. sigh I'll just go ahead and bet .some wasn't there before IE9 ...

[xixixao]

Surely we don't run the Cakefile in IE?

[alubbe]

@geoffreak you got it, that's how it works now!
I can also confirm that this

do co ->
  yield something()

co do ->
  yield something()

compiles to this

co(function*() {
  return (yield something());
})();

co((function*() {
  return (yield something());
})());

So if I understand you correctly, you want to be using the co do line.

[geoffreak]

@alubbe
I just wanted to make sure do co -> would work since this isn't pretty:

co(->*
  yield something()
)()

I know it works without any yield in regular coffeescript

[alubbe]

Yep, and now it works with generators - in both of the above examples it generated function* because yield was used in the function's body
So if you update to the new version you can use the prettier syntax 👍

[jashkenas]

Rad! Here we go.

[matthewwithanm]

@alubbe @jashkenas Thank you!!

[t3chnoboy]

Yay! Finally 🎉

[twincharged]

Score!!! Thank you, thank you, thank you @alubbe @jashkenas.

[SteveEisner]

This just made my day. @alubbe do you have a gittip or equivalent?

[jashkenas]

The test case that's in this patch is very nice — but if anyone wants to beef it up a little bit ... with examples that show yield operator precedence, yield detection inside of nested stuff inside the function, and maybe a trickier case of yield/yieldfrom — that would make me feel a little safer.

Also — if anyone has any concerns about the final state of the merged patch...

[vendethiel]

Nice :-).

[carlsmith]

:)

[darthmaim]

nice 👍

[schaitanya]

Finally 👍

[pgorzelany]

great !

[bjmiller]

I'm ecstatic! Now, uh, what version number should the release that contains this be? :)

[lazdmx]

nice!!

[lazdmx]

@jashkenas how about 1.8.1 release?

[alubbe]

my understanding of semver is that 1.8.1 would be a bugfix to 1.8.0, 1.9.0 would be new features that are backwards compatible and 2.0.0 would be new features that are backwards-incompatible. Seeing as coffeescript always reserved the yield keyword, this PR adds new features that are 100% backwards compatible, therefore it would make sense to go with 1.9.0.

[alubbe]

@SteveEisner @twincharged @matthewwithanm thank you for all the flowers! Happy to see this PR merged and that it will hopefully make everyone's code and life a bit nicer.

Also, thanks for offering to tip, but I'd feel better about something non-monetary. If you'd like you can follow me or something O_o''

[Starefossen]

@lazutkin @bjmiller The version should at least be 1.9.0 according to Semantic Versioning! And as @alubbe says, the yield keyword has been reserved since the first version of CoffeeScript so a major version bump for this feature isn't strictly necessary.

[vendethiel]

Not that @jashkenas believes in semver ;).

[michaelficarra]

This is nowhere near sufficient to detect yield in the body. This assumes the function has a completely flat structure. f (yield 0), (yield 1) will fail the test.

[barretron]

f (yield 0), (yield 1) fails because yield is used outside of a function definition. The parser works recursively, so nested structures will indeed pass the test. For example, x = -> f (yield 0), (yield 1) works correctly.

[michaelficarra]

I see, @body.contains walks the tree. I had thought it was shallow.

[taoeffect]

OMG. Congrats on finally getting this in!! Can't wait for 1.9.0! And phew! I was thinking I would have to abandon the language. So glad to see this, and can't wait to use it in production code with co.

[lbeschastny]

yield support works great, but there is still no way to create a non-yieldable generator in CS. This feature was actively discussed here, but discussion stopped after PR was merged.

So, any plans to support it?

I think, there should be some way to explicitly create a generator function. Otherwise, generators support seems incomplete.

[lydell]

It is already supported. http://coffeescript.org/#try:nonYieldingGenerator%20%3D%20-%3E%0A%20%20doSomething()%0A%20%20yield%20return

[lbeschastny]

@lydell thanx!