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

Can't pass dot/hidden files on command line #4828

Closed
radek-holy opened this issue Dec 29, 2015 · 50 comments

Comments

Projects
None yet
8 participants
@radek-holy
Copy link

commented Dec 29, 2015

Hello,
since JavaScript config files are supported now, can you please consider adding a (at least optional) way to ask eslint to lint also hidden/dot files (e.g. .eslintrc.js or .eslintrc.json with the JSON plugin) if a name to a directory is passed (e.g. eslint .)?

Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@eslintbot

This comment has been minimized.

Copy link

commented Dec 29, 2015

@radek-holy Thanks for the issue! 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.

@eslintbot eslintbot added the triage label Dec 29, 2015

@nzakas

This comment has been minimized.

Copy link
Member

commented Dec 29, 2015

Are you asking to include all dot files or just .eslintrc.js?

Note you can pass .eslintrc.js directly on the command line already.

@nzakas nzakas added enhancement cli evaluating and removed triage labels Dec 29, 2015

@radek-holy

This comment has been minimized.

Copy link
Author

commented Dec 29, 2015

I'm asking to include all dot files. At least optionally. It is related to the feature request #4829 (I'd like to test all the rc files of all the linters).

@michaelficarra

This comment has been minimized.

Copy link
Member

commented Dec 29, 2015

Can't you just pass **/.* explicitly? It's even easier than an option.

@radek-holy

This comment has been minimized.

Copy link
Author

commented Dec 29, 2015

Yes, sure, I can. This is just a request for a nice to have feature. I thought that because ESLint started to support JavaScript configs, potentially every user who uses .eslintrc.js would like to lint the config as well and thus every such user would welcome if ESLint did that automatically. And since I thought that it would sound questionable if ESLint would make just one exception, I thought that it could test all dot files. Actually, I failed to find an example where a user would not want to test a .*.js file... If you don't share my beliefs, feel free to close this (or change this request to something acceptable).

@DarkPark

This comment has been minimized.

Copy link

commented Mar 8, 2016

@nzakas, sadly seems explicit eslint ./.eslintrc.js doesn't work - I don't receive any errors/warnings from this file

@michaelficarra

This comment has been minimized.

Copy link
Member

commented Mar 8, 2016

@DarkPark Maybe there's nothing to warn about.

@ilyavolodin

This comment has been minimized.

Copy link
Member

commented Mar 8, 2016

@DarkPark Could you try to run the same command with --debug flag and check if the file is actually getting linted?

@DarkPark

This comment has been minimized.

Copy link

commented Mar 8, 2016

@michaelficarra, I deliberately made some :)

@DarkPark

This comment has been minimized.

Copy link

commented Mar 8, 2016

v2.3.0

@ilyavolodin, eslint --debug ./.eslintrc.js gives:

  eslint:cli Running on files +0ms
  eslint:ignored-paths Looking for ignore file in /home/dp/Projects/sdk/cjs/eslint-config +30ms
  eslint:ignored-paths Could not find ignore file in cwd +2ms
  eslint:cli-engine Linting complete in: 2ms +2ms
@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 9, 2016

@DarkPark can you rename to eslintrc.js and run with --debug so we can see your expected output?

@DarkPark

This comment has been minimized.

Copy link

commented Mar 9, 2016

@nzakas, sure, here it is:

eslint --debug ./eslintrc.js
  eslint:cli Running on files +0ms
  eslint:ignored-paths Looking for ignore file in /home/dp/Projects/sdk/cjs/eslint-config +28ms
  eslint:ignored-paths Could not find ignore file in cwd +1ms
  eslint:cli-engine Processing /home/dp/Projects/sdk/cjs/eslint-config/eslintrc.js +4ms
  eslint:cli-engine Linting /home/dp/Projects/sdk/cjs/eslint-config/eslintrc.js +1ms
  eslint:config Constructing config for /home/dp/Projects/sdk/cjs/eslint-config/eslintrc.js +1ms
  eslint:config Using .eslintrc and package.json files +0ms
  eslint:config Loading /home/dp/Projects/sdk/cjs/eslint-config/.eslintrc.js +2ms
  eslint:config-file Loading JS config file: /home/dp/Projects/sdk/cjs/eslint-config/.eslintrc.js +1ms
  eslint:config Using /home/dp/Projects/sdk/cjs/eslint-config/.eslintrc.js +187ms
  eslint:config Loading /home/dp/Projects/sdk/package.json +0ms
  eslint:config-file Loading package.json config file: /home/dp/Projects/sdk/package.json +1ms
  eslint:config-file Loading JSON config file: /home/dp/Projects/sdk/package.json +0ms
  eslint:config Merging command line environment settings +1ms
  eslint:config-ops Apply environment settings to config +0ms
  eslint:config-ops Creating config for environment commonjs +0ms
  eslint:cli-engine Linting complete in: 386ms +189ms

/home/dp/Projects/sdk/cjs/eslint-config/eslintrc.js
  19:9   warning  Extra space after key 'comma-dangle'               key-spacing
  19:25  warning  Missing space before value for key 'comma-dangle'  key-spacing
  19:26  error    Strings must use singlequote                       quotes

✖ 3 problems (1 error, 2 warnings)
@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 9, 2016

Thanks. So this looks like a bug, we should be linting anything passed on the command line. We should fix that.

@nzakas nzakas changed the title (optionally) lint dot/hidden files Can't pass dot/hidden files on command line Mar 9, 2016

@alberto

This comment has been minimized.

Copy link
Member

commented Mar 15, 2016

@nzakas So if we do eslint node_modules should we also lint that?

What about eslint . with a node_modules folder?

@michaelficarra

This comment has been minimized.

Copy link
Member

commented Mar 16, 2016

@alberto

So if we do eslint node_modules should we also lint that?

Yes because it was passed on the command line.

What about eslint . with a node_modules folder?

No because it wasn't passed on the command line.

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 17, 2016

Agree with @michaelficarra

@alberto

This comment has been minimized.

Copy link
Member

commented Mar 17, 2016

So, I guess we should also allow doing something like
eslint node_modules/some_module/some/deep/folder ?

Also, for reference, the current behaviour with hidden files is explicitly covered by our tests here.

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 18, 2016

Yup

@alberto

This comment has been minimized.

Copy link
Member

commented Mar 20, 2016

ok, one more question (sorry I keep coming at this, I'm trying to understand the reach and implications of this). What about glob patterns?

Should eslint **/*.js be expanded and run also on ignored folders (including node_modules and bower_components) ?
Should eslint **/.* be expanded and run on all hidden files?

@alberto

This comment has been minimized.

Copy link
Member

commented Mar 20, 2016

@radek-holy as a workaround, you can run eslint --no-ignore .eslintrc.js

@nzakas

This comment has been minimized.

Copy link
Member

commented Mar 30, 2016

@IanVS we have a bug where ! isn't working right now: #5728

We can wait until that gets resolved and then revisit this.

@IanVS

This comment has been minimized.

Copy link
Member

commented Mar 31, 2016

I did a little digging, and I think I figured out what's going on. When you pass in . as the argument to eslint, we convert it into a glob like /absolute/path/to/your/path/**/*.js. We then use node-glob to get a list of files to process. By default, node-glob will not match files and folders which begin with a . (https://github.com/isaacs/node-glob#dots).

So, while the !.* ignore pattern is working correctly, the dotfiles are not being considered for linting at all. A workaround for this is to specify two patterns when you lint, as so:

eslint . .*

I also think it would be safe for us to use the dot: true option in node-glob so that dot files are always considered, but will normally be ignored because of our default ignore rules. If the rest of the team is satisfied with this solution, I'd be happy to submit a PR for it.

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 5, 2016

@IanVS can you summarize what your proposal is at this point?

@nzakas nzakas added accepted and removed evaluating labels Apr 5, 2016

@alberto

This comment has been minimized.

Copy link
Member

commented Apr 5, 2016

He's proposing to change glob options in glob-util to make it consider dot files and folders. This won't change anything by default since they are ignored by eslint, but it will allow unignoring them in .eslintignore. I'm 👍 on that.

@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 6, 2016

@alberto it's not clear if he meant just that or that and a command line flag.

@IanVS can you clarify?

@IanVS

This comment has been minimized.

Copy link
Member

commented Apr 7, 2016

I no longer think it's necessary to include a command line option, since the same can be easily accomplished with a !.* pattern. Alberto's understanding was correct.

But, if we do this, dotfiles ending with .js (or specified extensions) will be linted when using --no-ignore. Is that acceptable?

@IanVS

This comment has been minimized.

Copy link
Member

commented Apr 7, 2016

Regarding my last statement above, I think this highlights a deeper problem, similar to #5547. Using --no-ignore should not unignore the default ignores (dependency folders and dotfiles).

So to be clear, my proposal is:

  1. Prevent dotfiles from being unignored by --no-ignore.
  2. Return dotfiles from glob-util as described in #4828 (comment).
@nzakas

This comment has been minimized.

Copy link
Member

commented Apr 7, 2016

Sounds good.

@IanVS IanVS self-assigned this Apr 11, 2016

@IanVS

This comment has been minimized.

Copy link
Member

commented Apr 11, 2016

My point number one above should be covered by #5820. After that is merged, I'll work on number two.

@IanVS

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

Sorry, I lost track of this. Will work on it shortly.

@alberto

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

I think we should wait on this until #6710 is solved.

@IanVS

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

@alberto, could you explain why? I know #6710 is an important enhancement, but this bug prevents use of ESLint altogether for some cases. And I don't see how the other issue is a blocker in any event.

@alberto

This comment has been minimized.

Copy link
Member

commented Aug 5, 2016

Sorry, you are right maybe it's not needed. I was planning to fix #6710 and then fix this, because I was concerned there could be a performance impact with hidden folders created by tools, but I don't have any numbers.

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.