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

add option to enable --quiet in eslint #340

Closed
jeffshaver opened this issue Oct 8, 2019 · 13 comments
Closed

add option to enable --quiet in eslint #340

jeffshaver opened this issue Oct 8, 2019 · 13 comments

Comments

@jeffshaver
Copy link

In eslint-loader, you have the option to pass quiet: true and it will only show errors in the output. When I was trying to switch over to this instead of eslint-loader, there is no option to pass the quiet option. You cannot pass this option via eslintOptions because the CLIEngine doesn't actually accept a quiet option.

@johnnyreilly
Copy link
Member

That sounds like a potentially useful option! Would you like to submit a PR? I suspect this is probably not too difficult to implement.

@jeffshaver
Copy link
Author

@johnnyreilly i could definitely look to help. I haven't dove into the repo. any suggestions on where to start?

@jeffshaver
Copy link
Author

@johnnyreilly looks like this is what we want: https://eslint.org/docs/developer-guide/nodejs-api#clienginegeterrorresults

And it looks like all i would need to do is add a quiet option to this plugin, and then check that new option after we call executeOnFiles. If true, set the lints variable to the result of getErrorResults. does that sound about right to you?

@jeffshaver
Copy link
Author

jeffshaver commented Oct 16, 2019

@johnnyreilly Actually, since we pass around a LintReport object most of the time, we would need to filter right before outputting to the console... One option is to put a filter here before the map:

...lint.messages.map(message =>

if message.severity !== 2 and the new option is true, then filter it out.

the other option is to use value.results if the option is false, and use the result of CLIEngine.getErrorResults(value.results) if the option is true

I am leaning toward the latter since eslint is already providing us a way to only get the error results

@johnnyreilly
Copy link
Member

johnnyreilly commented Oct 16, 2019

The latter seems like a better choice; why reimplement what ESLint already has. Question though; is this not configurable with the .eslintrc?

Also, could you expand a little on why a quiet option would be useful? It's not clear to me why you'd like to see warnings inside VS Code (or your editor of choice) but not in the console.

eslint/eslint#905

It's interesting that the option is supported in the CLI but not elsewhere. Wonder why?

https://github.com/webpack-contrib/eslint-loader/blob/master/README.md#quiet

ESLint loader implementation for context:

https://github.com/webpack-contrib/eslint-loader/blob/2dd8b43b2eacbe86b8a0bd984e0888d030263369/src/Linter.js#L99

@jeffshaver
Copy link
Author

@johnnyreilly so our specific use-case is that we have eslint-plugin-react-hooks and some other plugins that produce false-positives sometimes. In our case, 36 files have "false-positive" warnings. I don't want to never be able to find the warnings, but while I'm actively building, i want to get rid of the error level issues, without seeing 36 files worth of output that only contains warnings.

I think this would be useful for anyone who wants to see the errors actively during development, but wants to see the warnings in their IDE (less intrusive) and when they manually run eslint.

@johnnyreilly
Copy link
Member

Thanks for the clarification. I think it's probably a useful feature and it seems like implementation should be minimal in terms of complexity. If you'd like to put together a PR I'll take a look 🥰

@jeffshaver
Copy link
Author

@johnnyreilly how do you suggest testing this? I have the change (not that much code has changed). But the way its implemented, we only change the output going to the CLI, not the lint report itself (i can't just check the warning count).

@johnnyreilly
Copy link
Member

We have an integration test suite - probably best to provide a test which includes an error and a warning but only shows the error in the output.

jeffshaver pushed a commit to jeffshaver/fork-ts-checker-webpack-plugin that referenced this issue Oct 21, 2019
jeffshaver pushed a commit to jeffshaver/fork-ts-checker-webpack-plugin that referenced this issue Oct 21, 2019
jeffshaver pushed a commit to jeffshaver/fork-ts-checker-webpack-plugin that referenced this issue Oct 21, 2019
@jeffshaver
Copy link
Author

@johnnyreilly i put up a pr, but was having some difficulty with the integration test i was trying to write.

#346

Thanks

@johnnyreilly
Copy link
Member

Thanks @jeffshaver - I'll take a look but it won't be straight away (pretty snowed)

@jeffshaver
Copy link
Author

@johnnyreilly np. just let me know when you have time. as soon as i get feedback, ill try to update. appreciate your time

@piotr-oles piotr-oles mentioned this issue Apr 18, 2020
26 tasks
@piotr-oles
Copy link
Collaborator

@jeffshaver
Please try fork-ts-checker-webpack-plugin@alpha - I've published a new version which allows to filter out issues using configuration (issues options) 🚀
I will close this issue to clean-up the backlog :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants