anandthakker / doiuse

:bomb: Lint CSS for browser support against caniuse database.
MIT License
1.25k stars 51 forks source link

Add array of ignoring rules, per issue #21 #25

Closed sebastienbarre closed 9 years ago

sebastienbarre commented 9 years ago

Per discussion in #21, add array of ignoring rules, i.e.:

postcss(doiuse(opts)).process(css), where opts is:

{
  browsers: ['ie >= 8', '> 1%'] // an autoprefixer-like array of browsers.
  ignore: ['rem'], // an optional array of features to ignore
  onFeatureUsage: function(usageInfo) { } // a callback for usages of features not supported by the selected browsers
}

A test was added as well, for the PostCSS plugin.

However, neither stream.js or cli.js were updated. The issue here is that it would likely break the current API, a decision I'd rather you make. Right now, the stream signature is:

/**
 * @param {Array<string>} browsers  the browserslist browser selection
 * @param {string} [filename]  Filename for outputting source code locations.
 */
function stream (browsers, filename) {

It would have to be updated to support the ignore option (i.e. the array of features to ignore). Either:

function stream (browsers, ignore, filename) {

or:

function stream (options, filename) {

where options would have the browsers and ignore keys.

Since cli.js uses stream.js, it would have to be updated as well, from:

  process.stdin
    .pipe(doiuse(argv.browsers))
    .pipe(out)

to:

  process.stdin
    .pipe(doiuse(argv.browsers, argv.ignore))
    .pipe(out)

or:

  process.stdin
    .pipe(doiuse({browsers: argv.browsers, ignore: argv.ignore}))
    .pipe(out)

What do you think?

anandthakker commented 9 years ago

This looks great, @sebastienbarre.

Thanks for anticipating the API issue in stream -- I'd say let's go for the second approach you suggested, stream(options, filename), since that would stand up more flexibly to future additive changes.

sebastienbarre commented 9 years ago

Will do. I assume you are following semver, so I'll be bumping the version to 2.0.0 to reflect the breaking change, ok?

anandthakker commented 9 years ago

Yep, agreed this makes it 2.0.0.

Heh I managed to change the README out from under you here, hence the merge conflict. Should be an easy rebase.

sebastienbarre commented 9 years ago

All done (I think). Sorry, for the last empty merge commit, this always confuses me.

anandthakker commented 9 years ago

:+1: Looks great, thanks again @sebastienbarre

sebastienbarre commented 9 years ago

Thanks. FYI, quick heads up, I've another quick PR coming up for later today, to help with this one, related to #24