Adding a filter.restore options to end the stream when not used as a through stream

[nfroidure]

Hi,

Thanks for this plugin.

I ran into an issue will using it with StreamQueue for changing the files order in my builds.

Here is my task:

// Bower
gulp.task('build_bower', function() {
  var jsFilter = g.filter('**/*.js');
  var jqFilter = g.filter(['!**/dist/jquery.js']);
  var ngFilter = g.filter(['!**/angular.js', '!**/angular-mocks.js']);
  var ngStrictFilter = g.filter(['**/ui-utils.js']);
  var cssFilter = g.filter('**/*.css');
  return new StreamQueue({objectMode: true},
    jqFilter.restore(), // Never ended
    ngFilter.restore(), //never ended
    g.bowerFiles({
      paths: {
        bowerDirectory: src.vendors
      },
      includeDev: true || !prod // We currently depend on faker, so let it be always true for now..
    })
      .pipe(ngStrictFilter)
      .pipe(g.insert.wrap(
        ';\n(function(window, angular, undefined) {\n',
        '\n})(window, window.angular);\n'
      ))
      .pipe(ngStrictFilter.restore())
      .pipe(jqFilter) 
      .pipe(ngFilter)
      .pipe(jsFilter)
    ).pipe(g.cond(prod, g.streamify(g.concat.bind(null, 'libs.js'))))
    .pipe(getPathesOfStream(scripts, src.html))
    .pipe(jsFilter.restore())
    .pipe(cssFilter)
    .pipe(g.cond(prod, g.minifyCss))
    .pipe(g.cond(prod, g.concat.bind(null, 'vendors.css')))
    .pipe(getPathesOfStream(styles, src.html))
    .pipe(cssFilter.restore())
    .pipe(g.cond(prod, gulp.dest.bind(null, build.vendors)));
});

The task never completes since the restore stream are never ended. So my idea is to provide a "end" parameter allowing to end the restore stream when you do not use it as a PassTrough but as a simple Readable.

Something like:

jsFilter.restore({end: true})

What about it ? I can PR you if you agree on its API.

[sindresorhus]

Not a big fan to be honest. There has to be a better way than forcing all PassThrough streams to support this.

@mariocasciaro thoughts?

[mariocasciaro]

restore is not meant to be a simple Readable, at least based on the current design. It looks to me you are using restore as a NOT filter. In your example you can put a '!' at the beginning of your filter patterns and include directly the filter stream instead of the restore and you should get the same functionality, right?

[nfroidure]

@mariocasciaro i use streamqueue in order to be sure that files à reinjected in the pipeline will be in a determined order.

Beware that i do not want to preserve file order, but create an arbitrary order for my build concerns.

[mariocasciaro]

If you do something like this it should work:

source
.pipe(jqFilter)
.pipe(
  (new StreamQueue({objectMode: true},
    jqFilter.restore(),
    [...]
 )
)

Why you are not piping the filter before you pipe your StreamQueue? Is there any particular reason you are not doing this?

[nfroidure]

Ordering. I'm just filtering files, but i want to be able to use the filtered files in a special way that needs the restore streams to end by themself instead of ending with an external intervention.

My real world example is a bit complex. Let's make a simpler one:

var jsFilter = filter('**/*.js');
// Filtering them & doing something
gulp.src('src').pipe(jsFilter).pipe(...).pipe(gulp.dest('dest'));

// Doing something totally different with the filtered files
jsFilter.restore().pipe(...).pipe(gulp.dest('dest')); // this wil never end

IMO, filter.restore() beeing a readable stream is the best design since it is a file source, strictly. It is a PassThrough for convenience, i understand why, but it becomes annoying when you just want to use it for what it is, a file source.

So, regardless of the API, i think you should at least implement an option to do this. I can if you don't have time for that.

[mariocasciaro]

Yes, I think you have a point here. And using an option probably is also the best solution because it resembles the same option available for the pipe method. I would go for it @sindresorhus . You got a PR ready @nfroidure ?

[nfroidure]

Nope, but if you wish, i could do it.

[sindresorhus]

Alright. I get it now, with the simple example. Makes sense. Just wish it could adapt and be both Readable and PassThrough without an option.

PR welcome :) Would also be nice to have a new example in the readme showing this usecase.