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

Ignored files are excluded unless --force is passed on the CLI (fixes #813) #818

Merged
merged 1 commit into from Apr 24, 2014

Conversation

spadgos
Copy link
Contributor

@spadgos spadgos commented Apr 19, 2014

This PR implements the suggestion in #813 -- to exclude ignored files passed on the CLI.

To override this behaviour and lint all files passed in regardless of their ignore-state, I've added a flag --force.

This is still WIP, since there's no documentation of the feature yet, but I didn't want to spend time on that when the interface was not yet agreed upon.

One other thing which I think is missing is some form of alert to users when one or more ignored files were passed in. Without that, it would be quite possible that someone would run eslint my-ignored-file.js expecting it to be linted, and receive no errors.

As a point of reference, when ignored files are passed to git add, it will always exit immediately. I don't think this is useful for eslint though, since the purpose of this feature is to allow a list of files to be passed in and a subset to be linted.

@nzakas
Copy link
Member

nzakas commented Apr 19, 2014

Yeah, aborting doesn't seem correct. We could provide a warning for each ignored file, pushed onto the front of the messages that are passed to the formatter?

@spadgos
Copy link
Contributor Author

spadgos commented Apr 19, 2014

Yeah, a warning seems to fit nicely. I'll give it a shot.

@spadgos
Copy link
Contributor Author

spadgos commented Apr 19, 2014

Okay, here's an update. Still with no docs or enough tests, but hopefully should be enough to tell whether this is a good interface.

An example: assuming a directory with an ignored file called 'foo.js', the console output below shows three scenarios. First, specifying it on the CLI; second, adding the --force flag; and third, linting the whole directory.

$ eslint foo.js

foo.js
  0:0  warning  File ignored because of your .eslintignore file. Use --force to override

✖ 1 problem

$ echo $?
0

$ eslint --force foo.js

foo.js
  1:0   error  Missing "use strict" statement  strict

✖ 1 problem

$ echo $?
1

$ eslint .
$ echo $?
0

@spadgos
Copy link
Contributor Author

spadgos commented Apr 19, 2014

The tests are failing right now firstly because of a linting problem (ironically enough), but then because I didn't update all the formatters to allow for fatal: false. If what I've done with stylish.js is okay, then I'll port it over to the rest

@@ -59,6 +59,10 @@ module.exports = optionator({
type: "[String]",
description: "Specify environments."
}, {
option: "force",
type: "Boolean",
Copy link
Member

Choose a reason for hiding this comment

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

Can you add -f as a shortcut?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Already taken by the shortcut for --formatter. Any other suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

Dammit! No, never mind :)

@nzakas
Copy link
Member

nzakas commented Apr 19, 2014

Seems like we may be overloading the meaning of the fatal flag here. If we leave off fatal: false and there's no associated rule, won't this end up as a warning anyway? If not, I think that's a more appropriate approach to implement because who knows if we'd want to have other situations where we augment the messages.

@spadgos
Copy link
Contributor Author

spadgos commented Apr 20, 2014

Okay, I'll just change it to be more defensive in checking for the severity.

Ignored files are reported with a warning

Updates logic of formatters to treat non-fatal messages about non-existent rules as warnings.

Updates CLI documentation
@spadgos
Copy link
Contributor Author

spadgos commented Apr 24, 2014

Anything else you need from me on this one? Apart from the question about a shortcut name for --force, I think this one's ready to go.

@nzakas
Copy link
Member

nzakas commented Apr 24, 2014

LGTM. Sorry for the delay.

nzakas added a commit that referenced this pull request Apr 24, 2014
Ignored files are excluded unless --force is passed on the CLI (fixes #813)
@nzakas nzakas merged commit fd76e4d into eslint:master Apr 24, 2014
@spadgos spadgos deleted the always-exclude-ignored-files branch April 24, 2014 16:51
@eslint-deprecated eslint-deprecated bot locked and limited conversation to collaborators Feb 7, 2018
@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 Feb 7, 2018
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants