lazd / gulp-csslint

CSSLint plugin for gulp
MIT License
74 stars 12 forks source link

Add ability to write reports to file #34

Closed SimenB closed 8 years ago

SimenB commented 9 years ago

As discussed in #21

A way to write the report to file would be great, especially for reporters like junit and checkstyle. I came over https://github.com/juanfran/gulp-scss-lint which writes ti file, but not too cleanly... Other ideas for the API?

On a related note, I'd like to pass options in #29, we could put it in there? EDIT: If you look at the built in formatters (https://github.com/CSSLint/csslint/tree/master/src/formatters) they take an optionshash. Could just use that? I've already added it in the other PR, so for this it's just be the case of reusing that options-hash

/cc @joshuacc

joshuacc commented 9 years ago

:+1:

lazd commented 9 years ago

1. Second argument

As proposed in this pull request.

gulp.task('lint', function() {
  gulp.files('lib/*.css')
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', 'csslint-junit-report.xml'));

2. Separate reporters

To add this ability to gulp-jshint, folks started publishing versions of the reporters that write to files gulp-jshint-file-reporter.

gulp.src('./lib/*.js')
  .pipe(jshint())
  .pipe(jshint.reporter('gulp-jshint-file-reporter', {
    filename: __dirname + '/jshint-output.log'
  }));

3. Plugin option

In gulp-scss-lint, they're passing an option to the linter itself, which feels like Grunt, not gulp.

gulp.src(['**/*.scss'])
  .pipe(scsslint({
    'reporterOutput': 'scssReport.json'
  }));

4. Capture stdout

There is a gulp-log-capture, but it's not a very simple way to get the job done and will probably taint the reporter output by including the entire task log.

gulp.src('src/js/*.js')
  .pipe(jshint())
  .pipe(logCapture.start(console, 'log'))
  .pipe(jshint.reporter('jslint_xml'))
  .pipe(logCapture.stop('xml'))
  .pipe(gulp.dest('lint-reports'));
SimenB commented 9 years ago

1: In #29 an options-hash will be added as a second argument (should have been added in #21...), so we could piggy-back on that 2: Then we'd have to either call the built-in formatter again, or allow calling the reporter as well (feasible, I suppose) 3: Yeah, kinda. But as noted, the built-in formatters takes an options-hash, we could use that ourselves 4: That'll probably kill everything parseable about the report, thus rendering the feature virtually useless

lazd commented 9 years ago

@SimenB, check out how reporters work in gulp-escomplex -- they always create a File and send it down stream.

I talked with @phated, and another option is to always send a File down stream, and make printing configurable (on by default).

Using the options hash as a second argument, as you proposed, this would end up being something like:

gulp.task('lint', function() {
  gulp.files('lib/*.css')
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', { stdout: false })) // don't print to stdout
    .pipe(gulp.dest('results/'));

I suppose it's possible that we won't want to send the report downstream, but instead the files themselves... Is that a valid use case?

SimenB commented 9 years ago

The messes up linting files on the way to a final destinal (love those movies...), though, unless involving some fancy filtering and stuff. It'd work for my use-case atm, as I only use gulp for linting and testing, while webpack handles the whole "building and moving files" part, but I'm guessing there are a few people that lint the css-files on their way to some build or dist directory, and it would complicate their workflow.

How about if stdout === false we attach the formatted report to the file (instead of printing it), then provide a built-in "write-to-disk-reporter" (your option nr 2 earlier), and bundle that like the fail-reporter?

phated commented 9 years ago

Write to disk reporters are against gulp guidelines. You are not supposed to touch the file system outside src/dest. There are modules like gulp-filter and gulp-if for the scenarios you mentioned.

SimenB commented 9 years ago

Would it look something like this? I can't wrap my head around how to use filter or if to by-step the destructive operation. Am I missing something obvious?

gulp.task('css', function () {
  var cssFiles = gulp.src('src/styles/*.css');

  cssFiles
    .pipe(clone()) // gulp-clone
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', { stdout: false }))
    .pipe(gulp.dest('junit-report.xml'));

  return cssFiles
    .pipe(/*keep on piping the css-files, doing concat or whatever*/);
});

EDIT: I suppose we could ADD the file to the stream, then use filter/if to pipe it through gulp.dest. Does that make more sense, perhaps?

gulp.task('css', function () {
  var filter = gulpFilter('!csslint-report.xml');

  return gulp.src('src/styles/*.css');
    .pipe(csslint())
    .pipe(csslint.reporter('junit-xml', { stdout: false, fileName: 'csslint-report.xml' }))
    .pipe(filter())
    .pipe(gulp.dest('results/'))
    .pipe(filter.restore())
    .pipe(/*keep on piping the css-files, doing concat or whatever*/);
});
adamkingit commented 9 years ago

We have this need as well. An implementation similar to idea 1 that @lazd proposed on Aug 18 above would both allow the user to capture the output of any reporter, and allow more than one reporter to be chained like below: .pipe(csslint()) .pipe(csslint.reporter()) .pipe(csslint.reporter('junit-xml', {filePath:"test/lintcss.xml"})) .pipe(csslint.reporter('fail'))

How can I help with this request?

SimenB commented 9 years ago

@adamkingit Nag on me to complete #36 which includes the possibility to write to file.

EDIT: Or that seems complete (been a while since I worked on this). @lazd, anything holding up #36?

SimenB commented 8 years ago

Closing this, as it's covered in a cleaner way by #36