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

Task hangs if running in Coffeescript and there is a compile error #56

Closed
mhchen opened this issue May 6, 2014 · 6 comments
Closed

Comments

@mhchen
Copy link

mhchen commented May 6, 2014

If I get any kind of compile error in my Coffeescript, the task hangs and never returns control to Grunt. I did a bit of debugging and found that if the output option is set, it only calls finished() if stdout has some output, but a compile error means only stderr will have output and finished() never gets called. In my own script I am getting around this by introducing a delay of 1, but this feels like not the right way to handle this. Would it make sense to introduce a change with a stderr.on('data') that does the same thing as the stdout.on('data')?

Here's the code snippet I'm referring to:

    if (options.output) {
      server.stdout.on('data', function(data) {
        var message = "" + data;
        var regex = new RegExp(options.output, "gi");
        if (message.match(regex)) {
          finished();
        }
      });
    }

Thanks @ericclemmons for your maintenance of this project and for supporting Coffeescript even though you've said you don't use it very much yourself. It's much appreciated.

@paulpflug
Copy link
Collaborator

I think you are absolutely right, it should return on error.
I stumbled over this several times now but I didn't think about the problem, so thanks for pointing out ;)
I made a PR, see #57

@ericclemmons
Copy link
Owner

I just realized @mhchen called me out for not using Coffeescript ☕ :)

I'll switch, just so you don't think any lesser of me!!

@mhchen
Copy link
Author

mhchen commented May 6, 2014

Haha @ericclemmons not a callout at all. I know it's somewhat controversial, so it really was just a small show of appreciation for supporting a subset of your user base that you don't personally belong to

@ericclemmons
Copy link
Owner

After writing tons of nested promises that use function() { }.bind, I may have to switch ;)

ericclemmons added a commit that referenced this issue Aug 14, 2014
@ericclemmons
Copy link
Owner

Closed via #57

@ericclemmons
Copy link
Owner

Try out v0.4.18 to see if this fixes it!

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

No branches or pull requests

3 participants