spalger / gulp-jshint

JSHint plugin for gulp
MIT License
419 stars 65 forks source link

Add downstream option for fail-reporter #64

Closed Romelyus closed 10 years ago

Romelyus commented 10 years ago

Useful for gulp-plumber when you want only emit errors.

spalger commented 10 years ago

To me, fail reporter says "ensure that everything is valid, if not stop my build". I feel like you want something else... But I could be convinced otherwise.

I need more evidence that this is generally useful before I will merge it.

Romelyus commented 10 years ago

Event Error is not emitted by default and fail-reporter is required. But after an error i want to capture this event and continue processing using gulp-plumber. This behaviour useful for watching. Here is my code:

var gulp = require('gulp');
var plugin = require('gulp-load-plugins')();
var combine = require('stream-combiner');

var jsHintTasks = function() {
    return options.linting ? combine(
        plugin.jshint(),
        plugin.jshint.reporter('jshint-stylish'),
        plugin.jshint.reporter('fail')
    ) : plugin.util.noop();
};
var attachWatcher = function(taskName, source, pipeline, callback) {
    if (options.watch) {
        var taskIsDone;

        plugin.watch({
            glob: source,
            name: taskName
        }, function(files) {
            return files
                .pipe(plugin.plumber())
                .pipe(pipeline())
                .on('error', logError)
                .on('end', function() {
                    if (taskIsDone) {
                        logMessage(taskName, 'finished processing');
                    } else {
                        callback();
                        taskIsDone = true;
                    }
                });
        }).on('ready', function() {
            logMessage(taskName, 'is watching...');
        });
    } else {
        gulp.src(source)
            .pipe(plugin.plumber())
            .pipe(pipeline())
            .on('error', logError)
            .on('end', callback);
    }
};
var logError = function(error) {
    // here is my universal error handler
};
gulp.task('js', function(callback) {
    var taskName = 'js';

    var pipeline = function() {
        var templateFilter = plugin.filter(['**/*.tpl']);
        var jsFilter = plugin.filter(['**/*.js']);

        return combine(
            plugin.plumber(),
            plugin.cached(taskName, {optimizeMemory: true}),
            templateFilter,
            jsTemplatesTasks(),
            templateFilter.restore(),
            jsFilter,
            jsHintTasks(),
            plugin.debug(), // here i want my stream :)
            jsFilter.restore(),
            jsMinifyTasks(),
            plugin.remember(taskName),
            plugin.order(config.src.js.all, {base: '.'}),
            plugin.concat(config.build.js.name),
            gulp.dest(config.build.js.root)
        );
    };

    attachWatcher(taskName, config.src.js.all, pipeline, callback);
});
spalger commented 10 years ago

I went ahead and implemented a similar feature based on this comment. It has subtle differences from your implementation, but I think it should accomplish what you want.

Can you give it a shot and let me know if it plays well with plumber?

npm install spenceralger/gulp-jshint#unbuffered_fail

You might not need to pass the fail reporter anything, but if you could try both with and without {buffer: false} I would appreciate it.

Romelyus commented 10 years ago

It's works fine for me, but with {buffer: false} I received the same result as with {buffer: true}

Romelyus commented 10 years ago

What about PR?

spalger commented 10 years ago

Wow, sorry about that. I got busy with work and forgot to merge this. Just release 1.8.0.

While learning more about gulp-plumber I realized that I didn't need to force removal of files from the stream like I was. If no-one else is listening for the error event, then the stream will be unpiped and files won't flow anyway.

The buffer option is really just an optimization for people linting large code bases who don't really care about getting every error. For now, I'm going to leave it undocumented.