Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add advanced option globOptions to customise how matching works #179

Merged
merged 5 commits into from
Jun 1, 2017
Merged

Add advanced option globOptions to customise how matching works #179

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7ae9b19
Add advanced option globOptions to customise how matching works
georeith Jun 1, 2017
a5fc5cb
Forgot to assign globOptions
georeith Jun 1, 2017
72fbd96
Add globOptions config key to README
georeith Jun 1, 2017
a34f8a0
Don't use word list as its not really a list
georeith Jun 1, 2017
3a0feff
Typo in README globOptions description
georeith Jun 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/generateTasks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,19 @@

const minimatch = require('minimatch')

const readConfigOption = require('./readConfigOption')

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,
Copy link
Collaborator

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

Copy link
Contributor Author

@georeith georeith Jun 1, 2017 •
edited

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?

Copy link
Collaborator

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!

dot: true
})
}, globOptions))
const fileList = Object.keys(files).filter(filter).map(resolve)
return {
pattern,
Expand Down
21 changes: 21 additions & 0 deletions test/generateTasks.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -146,4 +146,25 @@ describe('generateTasks', () => {
]
})
})

it('should support globOptions specified in advanced configuration', () => {
const result = generateTasks(
{
globOptions: {
matchBase: false,
nocase: true
},
linters: {
'TeSt.*': 'lint'
}
},
files
)
const linter = result.find(item => item.pattern === 'TeSt.*')
expect(linter).toEqual({
pattern: 'TeSt.*',
commands: 'lint',
fileList: ['/root/test.js', '/root/test.css', '/root/test.txt']
})
})
})