-
-
Notifications
You must be signed in to change notification settings - Fork 425
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
Add advanced option globOptions to customise how matching works #179
Conversation
Codecov Report
@@ Coverage Diff @@
## master #179 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 5 5
Lines 65 67 +2
Branches 8 8
=====================================
+ Hits 65 67 +2
Continue to review full report at Codecov.
|
module.exports = function generateTasks(config, files) { | ||
const linters = config.linters !== undefined ? config.linters : config | ||
const resolve = file => files[file] | ||
return Object.keys(linters) | ||
.map((pattern) => { | ||
const commands = linters[pattern] | ||
const filter = minimatch.filter(pattern, { | ||
const globOptions = readConfigOption(config, 'globOptions', {}) | ||
const filter = minimatch.filter(pattern, Object.assign({ | ||
matchBase: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit-picking here but can't those defaults go into the 3rd argument at the L13? This should the whole Object.assign
here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@okonet that's not the same. According to readConfigOption
the default is only used if nothing is supplied. If a user sets { nocase: true }
in your suggestion it will also not use { matchBase: true, dot: true }
because it will never use the default.
When Object.assign()
is used later the user can override any keys without affecting the defaults (or override the defaults)
You could:
Object.assign({ matchBase: true, dot: true }, readConfigOption(config, 'globOptions', {}))
But it's not the same behaviour as:
readConfigOption(config, 'globOptions', { matchBase: true, dot: true })
unless I've misinterpreted how readConfigOption
works?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. Thanks for explanation!
One last thing that is missing is the documentation. Could you please update the README? |
@okonet Added 👍 |
Thanks for contribution! The new version will be released in a few minutes |
Adds the
globOptions
key to advanced config to allow customisation of how matching works.Will also solve (although it is not a bug in hindsight but not ideal either): #173