-
-
Notifications
You must be signed in to change notification settings - Fork 292
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
A way to override the default ignores #65
Comments
I would have thought that would work, and it definitely should. XO comes with good defaults, but everything should be overridable when needed. |
If anybody wants to take a crack at this #66 has a test prewritten for you! |
@jamestalmage Can you please tell me if ignore in XO config should totally override the default ignore? In current code it's appended to |
No, it should not. Using |
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? |
I propose this solution: 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: 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. |
If I specify an
|
Work around xojs/xo#65 by invoking XO from within the `test/fixture` directory. Fix linting issues. Most were harmless, but it did turn out that `babel-plugin-test-doubler.js` no longer doubled tests.
Work around xojs/xo#65 by invoking XO from within the `test/fixture` directory. Fix linting issues. Most were harmless, but it did turn out that `babel-plugin-test-doubler.js` no longer doubled tests.
@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 I believe that Clearly, this can add a lot of lines to If you’re 👍 with this, I can see and implement it. |
This allows to override the default ignored patterns. Fix xojs#65
@tusbar I don't like how it would completely opt out of all the default ignores. You would have to define a lot of |
The way I see it, there are 3 ways to do this: 1️⃣ What was suggested above: ignore the Consider the following tree:
I want Let’s assume that > glob.sync('**', { ignore: ['fixtures/**', '!fixture/success/**'] })
[] Negating patterns in So 1️⃣ is out of the question. @addobandre suggests that negating a pattern in "xo": {
"ignores": [
"!fixtures/**",
"fixtures/vendors/**",
"fixtures/break/**"
]
} This feels really awkward. I would rather override the "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:
In most of the packages that I lint with Finally, 3️⃣, I was suggesting a "xo": {
"patterns": [
"index.js",
"fixtures/success/**"
]
} 2️⃣ and 3️⃣ seem like reasonable options, though 2️⃣ is a severe breaking change. |
This is kind of annoying - had a script called $ 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? |
Hmm, I'm leaning towards just removing
That's a known issue: #238
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? |
Agreed.
Yes, #288 wasn't the correct fix.
I can't find any corresponding issue in Globby... I'm not sure what is the correct solution here. My solution was to remove all the negative globs from |
It was too opinionated and `bundle.js` can't always be assumed to be generated code that should not be linted. See: #65 (comment)
Done: 8e4f435
From https://github.com/mrmlnc/fast-glob/releases/tag/2.2.1:
|
@issuehuntfest has funded $80.00 to this issue. See it on IssueHunt |
@sindresorhus has rewarded $72.00 to @pvdlg. See it on IssueHunt
|
Reopening per #435 I sent an email to IssueHunt to revert the reward |
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:
But no luck
IssueHunt Summary
pvdlg has been rewarded.
Backers (Total: $80.00)
Submitted pull Requests
Tips
IssueHunt has been backed by the following sponsors. Become a sponsor
The text was updated successfully, but these errors were encountered: