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.

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

"eslint --ignore-path" should accept multiple file paths #9794

Closed
ratson opened this issue Jan 1, 2018 · 12 comments
Closed

"eslint --ignore-path" should accept multiple file paths #9794

ratson opened this issue Jan 1, 2018 · 12 comments
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion

Comments

@ratson
Copy link

ratson commented Jan 1, 2018

Current, eslint --ignore-path accept only one ignore file, it is useful if .gitignore and .eslintignore could be used together.

As of ESLint v4.14.0, --ignore-path command line argument is accepting path::String, is is proposed to change to [path::String], so addition files could be included.

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Jan 1, 2018
@aladdin-add aladdin-add added the needs info Not enough information has been provided to triage this issue label Jan 5, 2018
@eslint-deprecated
Copy link

Hi @ratson, thanks for the issue. It looks like there's not enough information for us to know how to help you.

If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

Requesting a rule change? Please see Proposing a Rule Change for instructions.

If it's something else, please just provide as much additional information as possible. Thanks!

@ratson
Copy link
Author

ratson commented Jan 9, 2018

Update issue body with the following, hope it is clear enough.

As of ESLint v4.14.0, --ignore-path command line argument is accepting path::String, is is proposed to change to [path::String], so addition files could be included.

@platinumazure platinumazure added enhancement This change enhances an existing feature of ESLint cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion and removed needs info Not enough information has been provided to triage this issue triage An ESLint team member will look at this issue soon labels Jan 9, 2018
@platinumazure
Copy link
Member

I'm 👍 to this enhancement as long as we have a clear path for how the ignores will be applied and whether the ignore patterns apply from the location of the ignore file or from the current working directory.

My suggestions on those points:

  • Process each file in order, with ignore paths from later files overriding/complementing those from earlier files
  • Each set of ignore patterns should be resolved relative to the location of the file containing them (to be consistent with current behavior for the single file case).

@ilyavolodin
Copy link
Member

ilyavolodin commented Jan 10, 2018

Second bullet point would add a bunch of additional complexity to the code to cover an edge case scenario. Not sure how necessary this is.
Ignore files used to cascade in the first implementation. It was full of bugs and missed cases, it also required caching in order to improve performance. I wouldn't want to go back to that, since in 4 years since it's been refactored, we haven't had many (if any) requests for support of multiple ignore files.

@graingert
Copy link
Contributor

graingert commented Jan 17, 2018

@ilyavolodin .gitignore + .eslintignore is a really common case. I've got a vendor directory in source control and I want to ignore my /build directories etc.

Currently both ignore files are in the root of the repo so it's less important that relative paths are preserved.

@maxmilton
Copy link

Ideally this should also be configurable via the config file (e.g. eslintrc.js) instead of just the CLI.

@platinumazure
Copy link
Member

@maxmilton See #3529 for previous discussion on why config files cannot have ESLint ignore paths/patterns. Basically, we read and resolve configurations only after knowing what directories to traverse, which means we have to know what directories to ignore before we start reading configuration files. So I don't think that's going to change.

@maxmilton
Copy link

@platinumazure thank you for clarifying, I hadn't seen that issue thread before. I'll continue to maintain multiple ignore files in my projects.

swernerx added a commit to sebastian-software/eslint-config-readable that referenced this issue May 11, 2018
…o .gitignore files which are then not working anymore. See issue: eslint/eslint#9794
@platinumazure
Copy link
Member

Seems discussion has stalled on this...

I would love to see support for multiple ignore files specified in --ignore-path. I would not like to see any form of auto-cascade/finding ignore files in ancestor directories.

Maybe it would be worth trying to prove the concept with a CLIEngine change first? E.g., add a method addIgnoreFile which would read the file and process the ignore patterns within, which would then be respected in a call to executeOnFiles. If we could prove something like that out, then it would be easy to add CLI options on top of that functionality.

@edele
Copy link

edele commented Jun 26, 2018

Here is an actual usecase.

I want eslint to only check files I can modify so I do --ignore-path .gitignore.
But additionally I don't want eslint to check my flow-typed annotations.

The only way I can do this now is to copy everything from .gitignore to .eslintignore and add flow-typed/* there. But then I have to always add lines to .gitignore and .eslintignore every time I want to ignore something.

This is kinda minor but I would love to know about a better way

@nzakas
Copy link
Member

nzakas commented Oct 24, 2018

Unfortunately, it looks like there wasn't enough interest from the team or community to implement this change. While we wish we'd be able to accommodate everyone's requests, we do need to prioritize. We've found that issues failing to be implemented after 90 days tend to never be implemented, and as such, we close those issues. This doesn't mean the idea isn't interesting or useful, just that it's not something the team can commit to.

@nzakas nzakas closed this as completed Oct 24, 2018
@borekb
Copy link

borekb commented Mar 13, 2019

For reference, Prettier currently cannot accept multiple --ignore-path options as well. It has been implemented in prettier/prettier#3737 but not merged in the end.

@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Apr 23, 2019
@eslint-deprecated eslint-deprecated bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Apr 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
archived due to age This issue has been archived; please open a new issue for any further discussion cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features enhancement This change enhances an existing feature of ESLint evaluating The team will evaluate this issue to decide whether it meets the criteria for inclusion
Projects
None yet
Development

No branches or pull requests

9 participants