feat(config): add `ignore` config option #385

kentcdodds · 2018-01-19T22:53:31Z

This allows users to prevent some files from being processed by lint-staged ever. This is useful if your project contains generated files that you commit to the repository, but don't want to process.

I know it's not great to commit generated files, but in this case it's really the best solution for me.

I didn't open an issue for this because I really need this anyway so if it's not merged then I'll just use my own fork.

This allows users to prevent some files from being processed by lint-staged ever. This is useful if your project contains generated files that you commit to the repository, but don't want to process.

codecov · 2018-01-19T22:58:22Z

Codecov Report

Merging #385 into master will increase coverage by 0.03%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #385      +/-   ##
==========================================
+ Coverage    98.1%   98.13%   +0.03%     
==========================================
  Files          10       10              
  Lines         211      215       +4     
  Branches       23       26       +3     
==========================================
+ Hits          207      211       +4     
  Misses          4        4

Impacted Files	Coverage Δ
src/getConfig.js	`100% <ø> (ø)`	⬆️
src/generateTasks.js	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e4f0f44...4b2f730. Read the comment docs.

sudo-suhas · 2018-01-20T02:24:14Z

Hey @kentcdodds, thanks for the PR 😄. I think overall, the feature addition makes sense. I have a minor suggestion. Instead of the multiple filter steps, how about we switch over to micromatch? This can filter files with multiple glob patterns and should be faster too.

kentcdodds · 2018-01-20T02:57:56Z

Sounds fine. What would the API look like to add an ignore? The same? Should we maybe do it in a different pull request in that case?

sudo-suhas · 2018-01-20T03:03:37Z

I was thinking the API would be the same. We can just concat the ignore patterns(with ! at the beginning) to the selection criteria and filter in a single step. However, if you have any suggestion for how it could be different, we could consider that.

kentcdodds · 2018-01-20T03:05:28Z

I don't mind making the change. Is this the only place minimatch is used in the codebase?

sudo-suhas · 2018-01-20T03:15:30Z

Yes, it is only used in generateTasks.js. It's also mentioned in a couple of places in the readme.

However, looks like there are very minor differences in the options for minimatch and micromatch. The following options are not present in micromatch:

So we might need to do this change in a separate PR and make a major release to denote the breaking change. I personally do not think these options would be used by anybody so it should be okay to make this change in a minor version. @okonet Could you take a call on this?

kentcdodds · 2018-01-20T07:07:01Z

Yes, I'd prefer to do that in a separate PR. Seems unrelated to the core feature I want to add here :)

luftywiranda13

Hi @kentcdodds, thanks for the PR 🎉

I have a small suggestion...

luftywiranda13 · 2018-01-20T10:12:01Z

README.md

@@ -129,6 +129,7 @@ To set options and keep lint-staged extensible, advanced format can be used. Thi
 * `chunkSize` — Max allowed chunk size based on number of files for glob pattern. This is important on windows based systems to avoid command length limitations. See [#147](https://github.com/okonet/lint-staged/issues/147)
 * `subTaskConcurrency` — `1` — Controls concurrency for processing chunks generated for each linter. Execution is **not** concurrent by default(see [#225](https://github.com/okonet/lint-staged/issues/225))
 * `globOptions` — `{ matchBase: true, dot: true }` — [minimatch options](https://github.com/isaacs/minimatch#options) to customize how glob patterns match files.
+* `ignore` - `['**/docs/**/*.js']` - array of glob patterns to entirely ignore from any task.


I think now it makes sense to sort items in options section alphabetically. So users can find what they need easier.

What do you think?

okonet · 2018-01-21T11:26:12Z

@kentcdodds thanks for your PR. We have a stalled (my fault!) PR that should be able to make the whole setup more flexible here #273

As @sudo-suhas mentioned we want to transition to micromatch and introduce array syntax to linters key so you'll be able to add multiple globs and ignore patterns in an obvious way. What do you think of the proposed syntax?

sudo-suhas · 2018-01-21T11:41:02Z

@okonet While having the option to include multiple file filter patterns would satisfy the requirement, I think the feature addition of global ignore is still justified. I think one of the good things is that it doesn't require any changes for existing configuration format. Also, do you think the minimatch ➜ micromatch switch requires a major version change?

okonet · 2018-01-21T12:23:13Z

Not sure about global ignore tbh if we want to have arrays anyways. Wouldn’t it just add more frustration to have both?

Switching glob engine shouldn’t be a major release unless it’s a breaking change.

sudo-suhas · 2018-01-21T12:32:08Z

Wouldn’t it just add more frustration to have both?

Wasn't the use-case of being able to ignore files our strongest motivation for introducing the multiple glob patterns option? Also, cascading rules are quite common(ex: eslint) so it shouldn't cause any issue in my opinion.

Switching glob engine shouldn’t be a major release unless it’s a breaking change.

Please see #385 (comment)

okonet · 2018-01-21T13:00:25Z

@sudo-suhas regarding mircomatch: we should do a major release then. No need to take any risks here.

Regarding the ignore option: if the only use case is to ignore files globally, when we should not over complicate things and introduce this global ignore patterns option. My concern is that next request will be "that's great but how do I ignore files only for this task" :)

But again, I'm leaving towards solving one problem at the time and it seems like @kentcdodds solution is good enough for now so let's go ahead with it.

Requested changes were done

sudo-suhas

LGTM

okonet · 2018-01-21T13:01:20Z

README.md

 * `concurrent` — *true* — runs linters for each glob pattern simultaneously. If you don’t want this, you can set `concurrent: false`
 * `chunkSize` — Max allowed chunk size based on number of files for glob pattern. This is important on windows based systems to avoid command length limitations. See [#147](https://github.com/okonet/lint-staged/issues/147)
+* `globOptions` — `{ matchBase: true, dot: true }` — [minimatch options](https://github.com/isaacs/minimatch#options) to 
+customize how glob patterns match files.
+* `ignore` - `['**/docs/**/*.js']` - array of glob patterns to entirely ignore from any task.


Can we add something like. Useful to skip linting of generated files.

okonet · 2018-01-21T13:06:40Z

Closes #310 (I guess?)

okonet · 2018-01-21T13:10:45Z

The only thing that just popped in my head is that this PR will prevent us from doing #277 which I'd really love to do since I never was a fan of having multiple config formats. ~~So, the question is: should the ignore go into linter's items config, perhaps?~~ Just realized it was the whole point of #273 discussion :(

sudo-suhas · 2018-01-21T13:15:44Z

#277 might make this obsolete in which case, we can drop it from the global options. Since it will be a breaking change, we can just ask users to move the ignore to each linter's options.

okonet · 2018-01-21T13:19:59Z

Not sure if this is a more desired approach in this case. I'd prefer to pull off the #273 instead, TBH. I'm wondering what @kentcdodds has to say if proposed changes would solve his use case.

Looks like I'll get some time to work on #62 and #273 soon!

kentcdodds · 2018-01-21T15:50:06Z

Looks like #273 was closed?

okonet · 2018-01-21T17:45:05Z

I will need to create a new PR but the discussion is still relevant.

kentcdodds · 2018-01-21T18:25:06Z

That looks like it would work fine for me. I just need a way to specify a glob of files I don't want processed.

kentcdodds · 2018-01-25T20:26:55Z

What's the status on this? Could we merge it as-is and then iterate to the improvements?

okonet · 2018-01-25T20:34:47Z

I still didn’t find to work on it yet. Sorry.

sudo-suhas · 2018-01-26T02:52:58Z

Could we merge it as-is and then iterate to the improvements?

I second this. Moreover, I have come to have reservations about 'Get rid of advanced config options'. We would be writing ourselves into a corner where every option would have to be at the linters level. This would restrict us in adding options which might make sense to have at the global level. To clarify, I support removing config options like subTaskConcurrency but I'd still like to leave the window open for any future requirements.

okonet · 2018-01-26T06:35:01Z

Okay let’s go forward with this PR then. @sudo-suhas please go ahead and merge it.

sudo-suhas · 2018-01-26T06:49:37Z

Merged and released in v6.1.0 🎉. Thanks again @kentcdodds!

kentcdodds · 2018-01-26T07:08:00Z

Super! Thank you!

okonet · 2018-01-26T09:12:46Z

Thanks @kentcdodds and @sudo-suhas for taking care of this!

evocateur · 2018-01-29T21:46:34Z

src/generateTasks.js

@@ -14,6 +14,12 @@ module.exports = function generateTasks(config, relFiles) {
  const normalizedConfig = getConfig(config) // Ensure we have a normalized config
  const linters = normalizedConfig.linters
  const globOptions = normalizedConfig.globOptions
+  const ignoreFilters = normalizedConfig.ignore.map(pattern => minimatch.filter(pattern))


This minimatch.filter() call isn't being passed globOptions. Is this intentional?

I'm also unable to ignore '.vscode/*.json' (whereas 'package.json' in the same ignores array works fine).

We should probably pass globOptions I guess 🤔

If we did, then your problem with .vscode would probably be resolved because you could specify the dot option.

I am not able to reproduce this issue locally:

λ git status On branch master Your branch is up to date with 'origin/master'. Changes to be committed: (use "git reset HEAD <file>..." to unstage) new file: .vscode/settings.json Changes not staged for commit: (use "git add <file>..." to update what will be committed) (use "git checkout -- <file>..." to discard changes in working directory) modified: .lintstagedrc λ cat .lintstagedrc { "linters": { "*.js": [ "eslint --fix", "git add" ], ".*rc": "jsonlint", "*.json": "jsonlint" }, "ignore": [".vscode/*.json"] } λ node . --debug lint-staged:bin Running `lint-staged@0.0.0-development` +0ms lint-staged Loading config using `cosmiconfig` +0ms lint-staged Successfully loaded config from `E:\Projects\repos\lint-staged\.lintstagedrc`: lint-staged { linters: lint-staged { '*.js': [ 'eslint --fix', 'git add' ], lint-staged '.*rc': 'jsonlint', lint-staged '*.json': 'jsonlint' }, lint-staged ignore: [ '.vscode/*.json' ] } +16ms lint-staged:cfg Normalizing config +0ms lint-staged:cfg Validating config +0ms Running lint-staged with the following config: { linters: { '*.js': [ 'eslint --fix', 'git add' ], '.*rc': 'jsonlint', '*.json': 'jsonlint' }, ignore: [ '.vscode/*.json' ], concurrent: true, chunkSize: 9007199254740991, globOptions: { matchBase: true, dot: true }, subTaskConcurrency: 1, renderer: 'verbose' } lint-staged Loaded scripts from package.json: lint-staged { precommit: 'node index.js', lint-staged cz: 'git-cz', lint-staged lint: 'eslint .', lint-staged 'lint:fix': 'yarn lint --fix', lint-staged pretest: 'yarn lint', lint-staged test: 'jest --coverage', lint-staged 'test:watch': 'jest --watch' } +16ms lint-staged:run Running all linter scripts +0ms lint-staged:run Resolved git directory to be `E:\Projects\repos\lint-staged` +15ms lint-staged:run Loaded list of staged files in git: lint-staged:run [ '.vscode/settings.json' ] +241ms lint-staged:gen-tasks Generating linter tasks +0ms lint-staged:cfg Normalizing config +272ms lint-staged:gen-tasks Generated task: lint-staged:gen-tasks { pattern: '*.js', lint-staged:gen-tasks commands: [ 'eslint --fix', 'git add' ], lint-staged:gen-tasks fileList: [] } +0ms lint-staged:gen-tasks Generated task: lint-staged:gen-tasks { pattern: '.*rc', commands: 'jsonlint', fileList: [] } +0ms lint-staged:gen-tasks Generated task: lint-staged:gen-tasks { pattern: '*.json', commands: 'jsonlint', fileList: [] } +0ms Running tasks for *.js [started] Running tasks for .*rc [started] Running tasks for *.json [started] Running tasks for *.js [skipped] → No staged files match *.js Running tasks for .*rc [skipped] → No staged files match .*rc Running tasks for *.json [skipped] → No staged files match *.json lint-staged linters were executed successfully! +271ms

Please take a look at your debug logs. It could be a configuration issue. If you continue to face the issue, please do create a new issue.

feat(config): add ignore config option

009d5e1

This allows users to prevent some files from being processed by lint-staged ever. This is useful if your project contains generated files that you commit to the repository, but don't want to process.

luftywiranda13 previously requested changes Jan 20, 2018

View reviewed changes

Kent C. Dodds added 2 commits January 20, 2018 11:57

docs(options): sort alphabetically

8aea25c

docs: fix globOptions

4b2f730

sudo-suhas approved these changes Jan 21, 2018

View reviewed changes

okonet reviewed Jan 21, 2018

View reviewed changes

okonet mentioned this pull request Jan 21, 2018

feat: Support glob filter array in linters #273

Closed

3 tasks

okonet mentioned this pull request Jan 21, 2018

Is negating groups supported? #310

Closed

sudo-suhas merged commit 5b7bc67 into lint-staged:master Jan 26, 2018

kentcdodds deleted the pr/ignore branch January 26, 2018 07:08

evocateur reviewed Jan 29, 2018

View reviewed changes

dependencies bot mentioned this pull request Feb 19, 2018

lint-staged versions available: 6.1.1, 6.1.0, 6.0.1 serlo/serlo-abc#256

Closed

okonet mentioned this pull request Dec 19, 2018

Allow defining a template for how paths should be passed to the command #138

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(config): add `ignore` config option #385

feat(config): add `ignore` config option #385

kentcdodds commented Jan 19, 2018

codecov bot commented Jan 19, 2018 •

edited

Loading

sudo-suhas commented Jan 20, 2018

kentcdodds commented Jan 20, 2018

sudo-suhas commented Jan 20, 2018

kentcdodds commented Jan 20, 2018

sudo-suhas commented Jan 20, 2018

kentcdodds commented Jan 20, 2018

luftywiranda13 left a comment

luftywiranda13 Jan 20, 2018

kentcdodds Jan 20, 2018

okonet commented Jan 21, 2018 •

edited

Loading

sudo-suhas commented Jan 21, 2018

okonet commented Jan 21, 2018

sudo-suhas commented Jan 21, 2018

okonet commented Jan 21, 2018 •

edited

Loading

sudo-suhas left a comment

okonet Jan 21, 2018

okonet commented Jan 21, 2018

okonet commented Jan 21, 2018 •

edited

Loading

sudo-suhas commented Jan 21, 2018

okonet commented Jan 21, 2018

kentcdodds commented Jan 21, 2018

okonet commented Jan 21, 2018

kentcdodds commented Jan 21, 2018

kentcdodds commented Jan 25, 2018

okonet commented Jan 25, 2018

sudo-suhas commented Jan 26, 2018

okonet commented Jan 26, 2018

sudo-suhas commented Jan 26, 2018

kentcdodds commented Jan 26, 2018

okonet commented Jan 26, 2018

evocateur Jan 29, 2018

kentcdodds Jan 29, 2018

sudo-suhas Jan 30, 2018 •

edited

Loading

feat(config): add ignore config option #385

feat(config): add ignore config option #385

Conversation

kentcdodds commented Jan 19, 2018

codecov bot commented Jan 19, 2018 • edited Loading

Codecov Report

sudo-suhas commented Jan 20, 2018

kentcdodds commented Jan 20, 2018

sudo-suhas commented Jan 20, 2018

kentcdodds commented Jan 20, 2018

sudo-suhas commented Jan 20, 2018

kentcdodds commented Jan 20, 2018

luftywiranda13 left a comment

Choose a reason for hiding this comment

luftywiranda13 Jan 20, 2018

Choose a reason for hiding this comment

kentcdodds Jan 20, 2018

Choose a reason for hiding this comment

okonet commented Jan 21, 2018 • edited Loading

sudo-suhas commented Jan 21, 2018

okonet commented Jan 21, 2018

sudo-suhas commented Jan 21, 2018

okonet commented Jan 21, 2018 • edited Loading

sudo-suhas left a comment

Choose a reason for hiding this comment

okonet Jan 21, 2018

Choose a reason for hiding this comment

okonet commented Jan 21, 2018

okonet commented Jan 21, 2018 • edited Loading

sudo-suhas commented Jan 21, 2018

okonet commented Jan 21, 2018

kentcdodds commented Jan 21, 2018

okonet commented Jan 21, 2018

kentcdodds commented Jan 21, 2018

kentcdodds commented Jan 25, 2018

okonet commented Jan 25, 2018

sudo-suhas commented Jan 26, 2018

okonet commented Jan 26, 2018

sudo-suhas commented Jan 26, 2018

kentcdodds commented Jan 26, 2018

okonet commented Jan 26, 2018

evocateur Jan 29, 2018

Choose a reason for hiding this comment

kentcdodds Jan 29, 2018

Choose a reason for hiding this comment

sudo-suhas Jan 30, 2018 • edited Loading

Choose a reason for hiding this comment

feat(config): add `ignore` config option #385

feat(config): add `ignore` config option #385

codecov bot commented Jan 19, 2018 •

edited

Loading

okonet commented Jan 21, 2018 •

edited

Loading

okonet commented Jan 21, 2018 •

edited

Loading

okonet commented Jan 21, 2018 •

edited

Loading

sudo-suhas Jan 30, 2018 •

edited

Loading