purifycss / gulp-purifycss

Removed unused CSS with the gulp build tool
MIT License
336 stars 21 forks source link

Handle Gulp Streams and Buffers Properly #6

Closed slickplaid closed 8 years ago

slickplaid commented 9 years ago

A quick update to the plugin so I can use it in my project. The tests could probably be expanded upon, but I didn't have much time to work on this yesterday. I'll take a look later today and see if I can make any further changes.

slickplaid commented 9 years ago

Shoot. Looks like someone beat me to the punch with a pull request.

Feel free to pick and merge what you like and don't like. I can expand upon this if you feel it might be useful in any way.

slickplaid commented 9 years ago

I'll see about abandoning this pull request and instead adding unit tests to the current state of the branch. Let me know if anything of use can be pulled from my work and I can integrate that as well.

Just from glancing at the other pull request, it looks like the other person switched the gulp.src() files to js and html inputs and the input for the plugin as the reference css files. I have it the opposite way. Not sure which is more semantically correct or the preferred method of the main developer.

Only reason for my way that I can see might be useful is to be able to stream edited CSS files against a defined source to spit out output. The other way would do the opposite. Depends on use cases, I suppose.

I could be completely wrong in my assessment of that, too. I've only glanced at the other person's source.

mrourke commented 9 years ago

Hi Evan, I'd like to add some basic unit tests that test the functionality of the plugin and that don't worry about all of the functionality of purify (those tests should be in purify's repo).

var purify = require('gulp-purify');

gulp.task('css', function() {
  return gulp.src('./public/app/example.css')
    .pipe(purify(['./public/app/**/*.js', './public/**/*.html']))
    .pipe(gulp.dest('./dist/'));
});

It currently takes in css files as the gulp.src and compares them to a glob of js and html to follow your logic and recommendations by gulp. Ideally, we don't want to make assumptions about any other plugins that might be used before or after this one.

slickplaid commented 9 years ago

Do you want me to write it assuming this pull request is abandoned and use the merged code from #2 ?

If that other pull you accepted at #2 seems to have resolved all issues, I can just alter my unit tests to work with that set of code instead and abandon this request.

alferov commented 9 years ago

So what is the final solution according this commit?

slickplaid commented 9 years ago

I'm somewhat confused on what the solution is as well. I can close (abandon) this code and issue another pull request with unit tests on the production branch for what code was merged on #2 . Just give the word. I'm kind of in a holding pattern until this pull request is resolved or abandoned.