Skip to content
This repository has been archived by the owner on Feb 26, 2024. It is now read-only.

Emit an event when there are clang-format warnings, so the user can act #2

Closed
wants to merge 1 commit into from

Conversation

alexeagle
Copy link
Contributor

No description provided.

gulp.task('check-format', function() {
return gulp.src('*.js')
.pipe(format.checkFormat('file'))
.on('warning', function(e) { process.stdout.write(e.message); process.exit(1) });
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be more convenient for users to call .pipe(format.checkFormat('file', {failOnFormatError: true}), and then have the pipeline emit an error? Or is that not possible thanks to Gulp?

@mprobst
Copy link
Contributor

mprobst commented Apr 2, 2015

LGTMs otherwise, in case the failOnFormatError thing isn't possible.

@alexeagle
Copy link
Contributor Author

It is possible to emit error instead, and add an optional parameter.
However, due to messy error handling, if you don't handle the error event it just breaks the pipeline. You need to have an on('error', {}) to properly deal with it - so it's longer than what I have here.
I also like the idea that user could do something else in the warning case, like run the clang-format command line themselves. So it's nice that we just emit 'warning' and it's up to them whether to treat as a warning or treat as an error.
Maybe it should be a gulp standard event?

@mprobst
Copy link
Contributor

mprobst commented Apr 2, 2015

I see, that makes sense.

I don't know what a gulp standard event would be. I think I've seen 'warn' somewhere, but have no idea if that's more standard. Overall gulp error handling seems like a lost cause, let's cut our investment and move to broccoli ASAP.

@alexeagle
Copy link
Contributor Author

thanks for review, merging as-is.

On Thu, Apr 2, 2015 at 10:24 AM Martin Probst notifications@github.com
wrote:

I see, that makes sense.

I don't know what a gulp standard event would be. I think I've seen 'warn'
somewhere, but have no idea if that's more standard. Overall gulp error
handling seems like a lost cause, let's cut our investment and move to
broccoli ASAP.


Reply to this email directly or view it on GitHub
#2 (comment)
.

@alexeagle
Copy link
Contributor Author

merged.

@alexeagle alexeagle closed this Apr 2, 2015
@alexeagle alexeagle deleted the emit_event branch April 2, 2015 17:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants