Why are the reporters for gulp-stylelint instead of just stylelint? about gulp-stylelint HOT 21 CLOSED

I created those reporters because at the time there was no alternative and I had to deliver a functioning linting system for a project at work in a limited time frame.

Also, consider the following code: gulp.src('*.css').pipe(foo);

The foo will not be given an array of files. It will be spoon-fed every matched file individually – first a.css, then b.css, then c.css, etc. Not all files at once.

So, what will happen if I pass stylelint-checkstyle-formatter as an option? If I understand it correctly, it will generate as many xml-formatted strings as there are files in the project. Not too useful.

By the way, there is already a checkstyle-formatter so there's no need to roll out another implementation based on string concatenation.

I'm open to change and welcome any suggestions.

from gulp-stylelint.

Thanks for suggested checkstyle-formatter. Didn't know about that --- will look into it.

The foo will not be given an array of files. It will be spoon-fed every matched file individually – first a.css, then b.css, then c.css, etc. Not all files at once.

That's a good point. Ideally we could share code between formatters but maybe that's not too realistic given this detail about gulp usage ...

At some point I will see if I can think of anything. And if you come up with any suggestions to modifying the stylelint formatters so that they work in this context, please let us know.

from gulp-stylelint.

When thinking about #5 and reading your code I realized that I'm not sure that there is actually a problem. If you use the Node API to process stylesheet code get a stylelint result object, one for each file; and you accumulate those result objects into an array as files stream through; then you finally run that array of results through a reporter --- doesn't that actually feed really nicely into the signature of a standard stylelint "formatter"?

Like, isn't it almost the same as what stylelint's Node API does when it receives a bunch of files? https://github.com/stylelint/stylelint/blob/master/src/standalone.js#L55

A stylelint formatter should receive an array of stylelint results and output something (http://stylelint.io/developer-guide/formatters/). In the plugin you are already accumulating such an array and sending it to your reporters. So if you modify the reporters to accept stylelint result objects instead of PostCSS warning objects (which you'd have to do anyway if you switch to the Node API), your "reporters" and stylelint's "formatters" should be interoperable, right?

It would be cool if this is right, but please correct me if I'm wrong :)

from gulp-stylelint.

I think that reporters can be entirely replaced with configuration objects. This will allow to avoid code duplication because the write logic will be contained only in gulp-stylelint.

import gulp from 'gulp';
import gulpStylelint from 'gulp-stylelint';
import textFormatter from 'stylelint-text-formatter';
import checkstyleFormatter from 'stylelint-checkstyle-formatter';

gulp.task('lint-css', function () {
  return gulp
    .src('src/**/*.css')
    .pipe(gulpStylelint({
      outputDir: 'reports/lint', // base folder
      failAfterError: true, // will emit error if results have errored
      reporters: [
        // text formatter will be used to write output to console:
        {formatter: textFormatter, show: true, save: false},
        // checkstyle formatter will be used to write output to reports/lint/checkstyle.xml:
        {formatter: checkstyleFormatter, show: false, save: 'checkstyle.xml'},
      ]
    });
});

from gulp-stylelint.

That's looking good! A couple of ideas:

• How about reportOutputDir instead of outputDir? I think that way less people will think that their CSS files are going to be output somewhere after the check.
• How about changing show to log, since that's a little more precise in describing what's happening?
• What do you think about having the standard test formatter run by default?

from gulp-stylelint.

How about reportOutputDir instead of outputDir? I think that way less people will think that their CSS files are going to be output somewhere after the check.

Good point.

How about changing show to log, since that's a little more precise in describing what's happening?

log: true might also mean "write output to a .log file". Maybe console: true?

What do you think about having the standard test formatter run by default?

Assuming you meant "standard text formatter". I'm hesitant to add this feature because I'd like to keep the code and the documentation minimalistic, and let the users decide which formatters they want or don't want to use.

from gulp-stylelint.

Maybe console: true?

Yeah, sounds good.

I'd like to keep the code and the documentation minimalistic

I understand. But am I right in thinking that this means that out of the box if you include gulp-stylelint in your pipeline you will get no report at all? I feel like that's not an ideal UX, especially not for some of the kind of devs that use stylelint (more into CSS than JS and put off by accumulating npm deps). Also, would it really involve that much additional complexity to the code, since the formatter would be drawn from stylelint?

from gulp-stylelint.

Sorry for responding late. Is there still interest in this?

To answer you concern about:

But am I right in thinking that this means that out of the box if you include gulp-stylelint in your pipeline you will get no report at all? I feel like that's not an ideal UX...

By default the pipeline will throw an error if any error-level issues are detected. Also, if I'm not mistaken, stylelint itself is a noop if you don't provide any rules, so the behavior is consistent.

from gulp-stylelint.

Yeah, still interested :) I had a flash of excitement for another possibility for a minute there (stylelint/stylelint#917) but I don't think that will work unless someone has a great idea for generically solving the very problem that your plugin solves for gulp --- which is to produce reports applicable to all files even though the runner processes each file individually.

Also, if I'm not mistaken, stylelint itself is a noop if you don't provide any rules, so the behavior is consistent.

That's true, but the CLI uses the text reporter as default, and the Node API uses the JSON reporter as default. I just think that for gulp-stylelint especially such a huge number of people are going to want the text reporter that it's worth a default. But it's not a big deal if you're not convinced, because it's easy enough to document what needs to be done :)

Just to recap where we are: I think we agreed that your new example configuration exemplified above was great with a couple of tweaks. That right?

from gulp-stylelint.

Just prepared a v1 release. There's an issue though – I can't find any text formatter, so it's pretty useless at the moment ^_^.

from gulp-stylelint.

I can help out. Should I look at that PR #6?

from gulp-stylelint.

If you wish so. I've tried it with your checkstyle formatter and it worked well.

As far as I know, there's no separate repository for a console or text formatter, so it needs to be created. I can temporarily reuse code from my console reporter.

from gulp-stylelint.

What about these? https://github.com/stylelint/stylelint/tree/master/src/formatters

The string formatter right now taps into postcss-reporter and does some customization. The JSON formatter is super straightforward.

from gulp-stylelint.

Oh, I guess that might work if the user can somehow import them.

from gulp-stylelint.

Since gulp-stylelint depends on styelint, and those are part of stylelint,
couldn't gulp-stylelint expose them, saving the need for a separate
package? E.g.

var gulpStylelint = require('gulp-stylelint');

gulpStylelint.formatter.string ...
gulpStylelint.formatter.json ...

On Tue, Mar 15, 2016 at 2:19 PM, Oleg Sklyanchuk [email protected]
wrote:

Oh, I guess that might work if the user can somehow import them.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#3 (comment)

from gulp-stylelint.

That is a possibility. For JSON formatter the user can simply pass JSON.stringify :) although that's kinda weird I think.

By the way, I've just noticed that, compared to these three formatters, checkstyle formatter expects a different input.

from gulp-stylelint.

The rule warnings should be the same (right?). But the deprecation and invalid option warnings have indeed changed and I have not yet modified checkstyle formatter. I removed those two types from the main warnings array so they could be treated specially.

from gulp-stylelint.

I guess it makes sense exposing available stylelint formatters on gulpStylelint.
Another possibility is to allow passing formatter names as strings.

reporters: [
  {formatter: 'verbose', console: true}, // same as gulpStylelint.formatters.verbose
  {formatter: gulpStylelint.formatters.json, save: 'report.json'}, // same as 'json'
  {formatter: myCustomFormatterFunction, save: 'custom-report.foo'} // not in stylelint, cannot be string
]

from gulp-stylelint.

Yeah, that would work, too.

from gulp-stylelint.

It's done.

from gulp-stylelint.

This has been released in 1.0.0. Feel free to re-open the issue if necessary. Thank you for your advice and contribution!

from gulp-stylelint.