Xo: A way to override the default ignores

Created on 3 Jan 2016 · 17Comments · Source: xojs/xo

Right now it's not possible to lint anything in a fixtures directory, even if you want to. There should be a way to enable that.

I tried this:

{
  "xo": {
    "ignore": [
      "!{test/,}fixture{s,}/**"
    ]
  }
}

But no luck

IssueHunt Summary

pvdlg has been rewarded.

Backers (Total: $80.00)

issuehunt ($80.00)

Submitted pull Requests

- #425 Find XO config based on linted file path

Tips

Checkout the Issuehunt explorer to discover more funded issues.

- Need some help from other developers? Add your repositories on IssueHunt to raise funds.

IssueHunt has been backed by the following sponsors. Become a sponsor

Rewarded on Issuehunt bug help wanted

Source

jamestalmage

All 17 comments

I would have thought that would work, and it definitely should. XO comes with good defaults, but everything should be overridable when needed.

sindresorhus on 3 Jan 2016

If anybody wants to take a crack at this #66 has a test prewritten for you!

jamestalmage on 3 Jan 2016

@jamestalmage Can you please tell me if ignore in XO config should totally override the default ignore? In current code it's appended to DEFAULT_IGNORE and then passed to globby which in turn passes it to glob.

jahson on 27 Jun 2016

Can you please tell me if ignore in XO config should totally override the default ignore?

No, it should not. Using ignore: '!test/fixtures/**' should not cause XO to start globbing node_modules

jamestalmage on 27 Jun 2016

Ok. This complicates things a bit. It looks like we should do some kind of glob expansion to actually override ignores and I doubt it's XO's area of responsibility. Or should it be done before passing to globby?

jahson on 28 Jun 2016

I propose this solution:
Negating a default ignore pattern would make xo remove it (and its negation) from the final ignores array.

It's not very flexible because it does no glob expansion, but it is easy to implement and gets the job done. To make it easier to override specific rules, we could just leave the default ignores expanded as such: [ '{tmp,temp}/**' ] -> [ 'tmp/**', 'temp/**' ].

Quick and dirty pseudocode:

// defaultIgnores <- default ignores array
// userIgnores <- the array defined by the user of ignore patterns
// ignores <- final ignores array

for(pattern in defaultIgnores) {
  const index = userIgnores.indexOf('!' + pattern)
  if(index !== -1) {
    delete userIgnores[index]
  } else {
    ignores.push(pattern)
  }
}

ignores = ignores.concat(userIgnores)

In conclusion, it would get xo a bit closer to expected behavior and leave room to reach expected behavior.

abread on 5 Oct 2016

If I specify an ignores: [ ... ] array of Strings to ignore, it gives me this error:

The `ignores` option requires the `filename` option to be defined

niftylettuce on 15 Aug 2017

@sindresorhus I’m having this issue as well. I’m thinking that, instead of doing what @addobandre suggests, we could implement a simple include/exclude (whitelist/blacklist) mechanism that we see everywhere (think webpack loaders, babel plugins, etc.).

Overriding the default ignored files could easily be done by specifying the files you want linted. For example having something like:

{
  "xo": {
    "src": [
      "src/**",
      "index.js",
      "{test/,}fixture{s,}/**"
    ]
  }
}

Specifying src would ignore ignores completely and run xo on the specified glob patterns.

I believe that src or files sound alright as the opposite of ignores, though a plural form is probably better. Whichever you prefer, I’m open to suggestions. Maybe patterns.

Clearly, this can add a lot of lines to package.json but most people probably use the default behavior anyway.

If you’re 👍 with this, I can see and implement it.

tusbar on 23 Nov 2017

@tusbar I don't like how it would completely opt out of all the default ignores. You would have to define a lot of ignores boilerplate in many projects. I prefer the approach of just individually overriding ignored paths.

sindresorhus on 24 Nov 2017

The way I see it, there are 3 ways to do this:

:one: What was suggested above: ignore the ignores by negating them.
:two: Completely override the default ignores and specify your own.
:three: Explicitly specify glob patterns to lint.

Consider the following tree:

├── index.js
├── index.min.js
└── fixtures
    ├── success
    │   └── foo.js
    ├── break
    │   └── color.js
    └── vendors
        └── something.js

I want index.js and foo.js to be linted.

Let’s assume that DEFAULT_IGNORE = ['fixtures/**', '**/*.min.js'].

> glob.sync('**', { ignore: ['fixtures/**', '!fixture/success/**'] })
[]

Negating patterns in glob’s ignore option is not a thing, and paths are ignored on first match.

So :one: is out of the question.

@addobandre suggests that negating a pattern in ignores could remove that pattern from the DEFAULT_IGNORE. It would break whenever the default ignores change, and still require users to define new ignores patterns to replace the removed one.

"xo": {
  "ignores": [
    "!fixtures/**",
    "fixtures/vendors/**",
    "fixtures/break/**"
  ]
}

This feels really awkward.

I would rather override the ignores completely (:two:) and have the following configuration:

"xo": {
  "ignores": [
    "**/*.min.js",
    "fixtures/vendors/**",
    "fixtures/break/**"
  ]
}

As long as the documentation specifies the default ignore patterns, I can port them over to my configuration if I need them.

As you said:

XO comes with good defaults, but everything should be overridable when needed.

In most of the packages that I lint with xo, the default ignores are great, but it gets tedious when you want to lint ignored files (like in ava).

Finally, :three:, I was suggesting a patterns or files option that is mutually exclusive with ignores, where you explicitly specify all the files that will be linted (just like you do when you use the command line).

"xo": {
  "patterns": [
    "index.js",
    "fixtures/success/**"
  ]
}

:two: and :three: seem like reasonable options, though :two: is a severe breaking change.

tusbar on 25 Nov 2017

This is kind of annoying - had a script called bundle.js and it was just being silently ignored. Even an explicit xo bundle.js invocation didn't yield anything.

$ mv bundle2.js bundle.js
$ clear
$ echo 'asdfjkaskdjfkasjdf herp derppppppppppp' > bundle.js
$ node_modules/.bin/xo bundle.js
$ mv bundle.js bundle2.js
$ node_modules/.bin/xo bundle2.js

  bundle2.js:1:20
  ✖  1:20  Parsing error: Unexpected token herp

  1 error

Took me 30 minutes to figure out what was going on and that I had a bunch of swallowed linter errors...

Can we re-open #288 as a workaround until upstream Globby gets patched?

Qix- on 23 Jul 2018

had a script called bundle.js

Hmm, I'm leaning towards just removing bundle.js from default ignores. I've never used that naming myself and it feels too opinionated. The default ignores should be something you would ignore 95% of the time. @pvdlg Thoughts?

Even an explicit xo bundle.js invocation didn't yield anything.

That's a known issue: https://github.com/xojs/xo/issues/238

Can we re-open #288 as a workaround until upstream Globby gets patched?

I would be in favor of just fixing it in Globby since that's easier than generally fixing it in Globby.

@pvdlg Do you remember which Globby issue this is about?

sindresorhus on 24 Jul 2018

❤1

Hmm, I'm leaning towards just removing bundle.js from default ignores.

Agreed.

I would be in favor of just fixing it in Globby since that's easier than generally fixing it in Globby.

Yes, #288 wasn't the correct fix.

Do you remember which Globby issue this is about?

I can't find any corresponding issue in Globby... I'm not sure what is the correct solution here.
The problem is that both node-glob and fast-glob do not handle ignore globs together. So setting ignore to [!**/bundle.js, **/bundle.js] still ignore **/bundle.js.

My solution was to remove all the negative globs from ignores, reverse them and add them to the list of patterns. I think that I hit some edges cases or performances issues doing so. I can't remember exactly.