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

Docs: Clarify implicit ignore behavior for dotfiles #12600

Merged
merged 1 commit into from Jul 3, 2020

Conversation

snhardin
Copy link
Contributor

What is the purpose of this pull request? (put an "X" next to item)

[ ] Documentation update
[X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Addresses issue #12348

What changes did you make? (Give an overview)
This is a small pull request that changes the logic for displaying an error message when a file is ignored by implicit ignore rules. The regular expression for detecting a dotfile was expanded to look for a dotfile in any part of the path rather than just the end of the path. Additionally, documentation that explains the nuances of the implicit ignore rules and a test for the new behavior introduced in this pull request have been added.

Is there anything you'd like reviewers to focus on?
None in particular. This pull request is small enough that I think someone knowledgeable can give good feedback on all aspects (code changes, documentation, testing).

@eslint-deprecated eslint-deprecated bot added the triage An ESLint team member will look at this issue soon label Nov 25, 2019
@kaicataldo kaicataldo added documentation Relates to ESLint's documentation accepted There is consensus among the team that this change meets the criteria for inclusion and removed triage An ESLint team member will look at this issue soon labels Nov 25, 2019
@kaicataldo kaicataldo added the enhancement This change enhances an existing feature of ESLint label Dec 20, 2019
@kaicataldo
Copy link
Member

Sorry for the delay on our end. It looks to me like the accepted issue was just the documentation change. Can we scope this PR to just improving documentation and make a new issue for the bug fix?

@snhardin
Copy link
Contributor Author

Sure. I'll remove the bug fix and just include the documentation changes. Have you made a new issue already?

@kaicataldo
Copy link
Member

Have you made a new issue already?

I don't think we have an issue for the code changes. You can feel free to create a new proposal for that!

@snhardin
Copy link
Contributor Author

snhardin commented Feb 1, 2020

I reduced this PR down to documentation changes. New issue + PR for code changes will come in a day or two.

@nzakas
Copy link
Member

nzakas commented Apr 23, 2020

@snhardin sorry we lost track of this. Are you interested in fixing the merge conflicts so we can look at merging this?

@snhardin
Copy link
Contributor Author

Certainly. I'll have them fixed in the next couple of days.

@nzakas
Copy link
Member

nzakas commented Apr 30, 2020

Thanks!

@snhardin
Copy link
Contributor Author

snhardin commented May 8, 2020

Done.


For example, placing the following `.eslintignore` file in the current working directory will not ignore `node_modules/*` and ignore anything in the `build/` directory except `build/index.js`:
* `node_modules/` as well as `bower_components/` directories are ignored.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default ignore patterns have changed in v7.0.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be fixed in latest push.


There are also some exceptions to these rules:

* If the path to lint is a glob pattern or directory path and contains a Dotfolder, all Dotfiles and Dotfolders will be linted. This includes subDotfiles and subDotfolders that are buried deeper in the directory structure.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* If the path to lint is a glob pattern or directory path and contains a Dotfolder, all Dotfiles and Dotfolders will be linted. This includes subDotfiles and subDotfolders that are buried deeper in the directory structure.
* If the path to lint is a glob pattern or directory path and contains a Dotfolder, all Dotfiles and Dotfolders will be linted. This includes sub-dotfiles and sub-dotfolders that are buried deeper in the directory structure.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.


```text
# node_modules/* is ignored by default, but can be added using !
# All .git/* Dotfiles as well as /node_modules/* and /bower_components/* in the project root are ignored by default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically, all dotfiles and dotfolders (except for .eslintrc.) are ignored, not just `.git/.

docs/user-guide/configuring.md Show resolved Hide resolved
docs/user-guide/configuring.md Show resolved Hide resolved
@nzakas
Copy link
Member

nzakas commented Jun 11, 2020

@snhardin there are some requested changes on this pull request. Please let us know if you're still working on the changes or if you'd like someone else to pick up this work. Thanks!

@nzakas nzakas changed the title Fix: Clarify implicit ignore behavior for dotfiles Docs: Clarify implicit ignore behavior for dotfiles Jun 11, 2020
@snhardin
Copy link
Contributor Author

Apologies! Did not see the emails for the requested changes. Will get on those asap.

@snhardin
Copy link
Contributor Author

Pushed some changes. Let me know what you think!

Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple editing requests, but otherwise LGTM. Thanks for working on this!


For example, `eslint .config/my-config-file.js --no-ignore` will cause `my-config-file.js` to be linted. It should be noted that the same command without the `--no-ignore` line will not lint the `my-config-file.js` file.

* Whitelist and blacklist rules specified via `--ignore-pattern` or `.eslintignore` are prioritized above implicit ignore rules.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Whitelist and blacklist rules specified via `--ignore-pattern` or `.eslintignore` are prioritized above implicit ignore rules.
* Allowlist and denylist rules specified via `--ignore-pattern` or `.eslintignore` are prioritized above implicit ignore rules.

docs/user-guide/configuring.md Show resolved Hide resolved

* Whitelist and blacklist rules specified via `--ignore-pattern` or `.eslintignore` are prioritized above implicit ignore rules.

For example, in this scenario, `.build/test.js` is the desired file to whitelist. Because all Dotfolders and their children are ignored by default, `.build` must first be whitelisted so that eslint because aware of its children. Then, `.build/test.js` must be explicitly whitelisted, while the rest of the content is blacklisted. This is done with the following `.eslintignore` file:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
For example, in this scenario, `.build/test.js` is the desired file to whitelist. Because all Dotfolders and their children are ignored by default, `.build` must first be whitelisted so that eslint because aware of its children. Then, `.build/test.js` must be explicitly whitelisted, while the rest of the content is blacklisted. This is done with the following `.eslintignore` file:
For example, in this scenario, `.build/test.js` is the desired file to allowlist. Because all Dotfolders and their children are ignored by default, `.build` must first be allowlisted so that eslint becomes aware of its children. Then, `.build/test.js` must be explicitly allowlisted, while the rest of the content is denylisted. This is done with the following `.eslintignore` file:

For example, in this scenario, `.build/test.js` is the desired file to whitelist. Because all Dotfolders and their children are ignored by default, `.build` must first be whitelisted so that eslint because aware of its children. Then, `.build/test.js` must be explicitly whitelisted, while the rest of the content is blacklisted. This is done with the following `.eslintignore` file:

```text
# Whitelist 'test.js' in the '.build' folder
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
# Whitelist 'test.js' in the '.build' folder
# Allowlist 'test.js' in the '.build' folder

✖ 1 problem (0 errors, 1 warning)
```

This messages occurs because normally, this file would be ignored by ESLint's implicit ignore rules (as mentioned above). A negated ignore rule in your `.eslintignore` file would override the implicit rule and reinclude this file for linting. Additionally, in this specific case, `--no-ignore` could be used to lint the file as well.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
This messages occurs because normally, this file would be ignored by ESLint's implicit ignore rules (as mentioned above). A negated ignore rule in your `.eslintignore` file would override the implicit rule and reinclude this file for linting. Additionally, in this specific case, `--no-ignore` could be used to lint the file as well.
This messages occurs because, normally, this file would be ignored by ESLint's implicit ignore rules (as mentioned above). A negated ignore rule in your `.eslintignore` file would override the implicit rule and reinclude this file for linting. Additionally, in this specific case, `--no-ignore` could be used to lint the file as well.

@snhardin
Copy link
Contributor Author

Added the comma after "because" and replaced all instances of "whitelist" and "blacklist."

Copy link
Member

@kaicataldo kaicataldo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@kaicataldo kaicataldo merged commit 825a5b9 into eslint:master Jul 3, 2020
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Dec 31, 2020
@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 Dec 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion documentation Relates to ESLint's documentation enhancement This change enhances an existing feature of ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clarification on behavior of dot directories
4 participants