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

Fixes #51. Prevents hard failure and lets tasks like watch keep running #52

Closed
wants to merge 1 commit into from
Closed

Conversation

eddiemonge
Copy link
Contributor

No description provided.

@sapegin
Copy link
Contributor

sapegin commented Aug 24, 2013

Is grunt watch --debug not enough to solve this problem?

@eddiemonge
Copy link
Contributor Author

no, thats not the point. If grunt watch is running and stylus has an issue, like not finding a file or a problem with the parsing the code, it kills watch. Other tasks report the error and keep watch running. Having to see the issue, fix it, and then restart the watch command is one step too many.

@sapegin
Copy link
Contributor

sapegin commented Aug 24, 2013

Never have problems with --debug.

In my opinion tasks should fail if there are some problems and if they are not started with --debug option.

@martinheidegger
Copy link

@sapegin There are different sort of problems. Some problems are unfixable (broken grunt config settings for example or missing important executables) that will not be fixed unless you restart grunt. However, other problems are fixable without changing the grunt settings (i.e. by fixing the .styl file). In the grunt-sass task any problem it will output that problem in the browser as well as in the command line and watch continues to work - very comfortable. (I agree with @eddiemonge +1)

@hurrymaplelad
Copy link
Contributor

I'm a little confused. I agree with @eddiemonge that grunt-contrib-stylus should play nice with grunt-contrib-watch, and with @sapegin that tasks should fail when they hit errors. Failing on errors usually means calling grunt.warn. I've seen two strategies for playing nice:

  1. grunt-shell has an explicit failOnError option and avoids calling grunt.warn when it's set.
  2. grunt-contrib-watch has a forever option that clobbers grunt.warn

But the changes in this PR don't prevent grunt.warn from being called (which leads to process.exit), and they seem to introduce a potential silent error. Maybe a test would help me understand when we expect stylus.render to throw instead of calling its callback with an error, and why we would want to squash that error.

callback(css, null);
}
});
} catch (e) {}
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we grunt.fail.warn here?

@vladikoff
Copy link
Member

gruntjs/grunt#1163

@eddiemonge eddiemonge closed this Feb 18, 2015
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.

None yet

5 participants