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

Negated ignore pattern doesn't work when file previously ignored with '**' rule #5979

Closed
jsnajdr opened this issue Apr 27, 2016 · 14 comments

Comments

Projects
None yet
5 participants
@jsnajdr
Copy link

commented Apr 27, 2016

What version of ESLint are you using?
2.8.0

What parser (default, Babel-ESLint, etc.) are you using?
default

What did you do? Please include the actual source code causing the issue.

  1. Create a new project with one file in a subdirectory: server/a.js
  2. Add this to your .eslintignore:
server/**
!server/a.js
  1. Run eslint server/a.js

What did you expect to happen?
The a.js file is linted, as it is explicitly unignored in the .eslintignore file.

What actually happened?
The a.js is ignored - this is the raw output:

$ eslint --debug server/a.js 
  eslint:cli Running on files +0ms
  eslint:ignored-paths Looking for ignore file in /Users/jsnajdr/src/lintbug +30ms
  eslint:ignored-paths Loaded ignore file /Users/jsnajdr/src/lintbug/.eslintignore +1ms
  eslint:ignored-paths Adding /Users/jsnajdr/src/lintbug/.eslintignore +0ms
  eslint:glob-util Creating list of files to process. +1ms
  eslint:cli-engine Linting complete in: 5ms +1ms

/Users/jsnajdr/src/lintbug/server/a.js
  0:0  warning  File ignored because of a matching ignore pattern. Use --no-ignore to override

✖ 1 problem (0 errors, 1 warning)

This bug seems to happen only when the ** pattern is used. When I change the above ignore rule to server/*.js, then the ! rule works correctly and unignores the a.js file.

@eslintbot eslintbot added the triage label Apr 27, 2016

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 27, 2016

Confirmed. Looks like an issue in node-ignore. kaelzhang/node-ignore#21

@kaelzhang

This comment has been minimized.

Copy link
Contributor

commented Apr 28, 2016

@jsnajdr reinstall eslint to upgrade node-ignore to 3.1.2 to see if it is ok now?

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

Thanks @kaelzhang, confirmed it works with 3.1.2

@alberto alberto removed the blocked label Apr 28, 2016

alberto added a commit that referenced this issue Apr 28, 2016

@jsnajdr

This comment has been minimized.

Copy link
Author

commented Apr 28, 2016

@kaelzhang The bug is only partially fixed with 3.1.2. With the following .eslintignore:

server/**
!server/a.js
!server/x/a.js

file server/a.js is not ignored any more, but server/x/a.js still is.

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

Oh, good catch @jsnajdr

@alberto alberto added the blocked label Apr 28, 2016

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

@jsnajdr Now that I think of it, for that to work, you would need to do:

server/**
!server/a.js
!server/x
!server/x/a.js

Does this work?

@jsnajdr

This comment has been minimized.

Copy link
Author

commented Apr 28, 2016

@alberto yes, this workaround works. But it shouldn't be needed - !server/x/a.js should just work.

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

@jsnajdr It's not a workaround. git works the same way

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

@jsnajdr Maybe you meant this?

server/**/*.js
!server/a.js
!server/x/a.js

I tested this and it also works now :)

In your previous example, note server/** also ignores server/x, and since you can't unignore a file in an ignored folder, you need to unignore the folder itself first. See https://git-scm.com/docs/gitignore for details

@jsnajdr

This comment has been minimized.

Copy link
Author

commented Apr 28, 2016

@alberto You're right, the gitignore docs state that "It is not possible to re-include a file if a parent directory of that file is excluded.", for performance reasons.

But this means that I can't put a whole source directory into .eslintignore (because it's not eslint clean and I don't want to break my builds) and the enable cleaned files one by one. We do this in Firefox (see here). Now I'd like to add an exception for !devtools/server/actors/webbrowser.js and override the devtools/server/** pattern, but I can't. I can't add !devtools/server/actors either, because that would unignore the whole actors directory, not just one specific file.

Such a use case doesn't exist in Git - nobody probably needs to cherry-pick files deep inside an ignored tree and add them to version control - but it's useful for ESLint. So maybe having exactly the same semantics as .gitignore is not appropriate for .eslintignore.

Seems I'll have to workaround this by ignoring the devtools/server subdirectories one by one, like we already do with devtools/shared. Or with the server/**/*.js trick.

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 28, 2016

Yes, I am afraid you'll have to especify the extension so folders don't get ignored or do this:

devtools/server/**
!devtools/server/actors
devtools/server/actors/**
!devtools/server/actors/webbrowser.js

I agree it's not ideal, but we use node-ignore for ignoring/unignoring files, and it adheres to the gitignore spec, so it would be difficult for us to deviate from that. You can open a different issue if you want to propose that anyways.

@jsnajdr

This comment has been minimized.

Copy link
Author

commented Apr 28, 2016

I think I now fully understand what's happening and why and I can adapt to that :) As for me, this issue can be closed.

@kaelzhang

This comment has been minimized.

Copy link
Contributor

commented Apr 29, 2016

[...] So maybe having exactly the same semantics as .gitignore is not appropriate for .eslintignore.

It is a good thought.

But guys are using --ignore-path .gitignore for their custom ignore rules. The purpose of doing this is to ignore files as the git does. If we don't filter files as .gitignore does, there might be unintended side effects.

The key question is that whether we should follow the "It is not possible to re-include a file if a parent directory of that file is excluded." of the .gitignore spec.

Or we might as well add a new command line option to provide a different behavior of the ignoring system.

But what we are discussing about might be another issue, however :)

@alberto alberto removed the blocked label Apr 29, 2016

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Apr 29, 2016

We recently switched our ignore processing from minimatch to node-ignore because our users were asking to follow standard gitignore conventions, so it's very unlikely that we will want to break compatibility again.
Closing, as it looks like original issue has been resolved.

ilyavolodin added a commit that referenced this issue Apr 29, 2016

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.