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 for --force-exclusion to ruby-rubocop checker? #1348

Closed
mmcnl opened this Issue Oct 25, 2017 · 8 comments

Comments

Projects
None yet
4 participants
@mmcnl

mmcnl commented Oct 25, 2017

As discussed in rubocop-hq/rubocop#893 (comment) when running rubocop from a linter such as flycheck, it is impossible to ignore a particular file or directory from the usual rubocop.yml config file. To address this, the rubocop project added the --force-exclusion command line option (rubocop-hq/rubocop#922). Would it be possible to add --force-exclusion to flycheck's default invocation of rubocop, or to add a flycheck var to enable that option?
Thanks so much for your work on this great tool!

@fmdkdd

This comment has been minimized.

Member

fmdkdd commented Oct 25, 2017

Would it be possible to add --force-exclusion to flycheck's default invocation of rubocop, or to add a flycheck var to enable that option?

Looking at the discussion in rubocop-hq/rubocop#893 (comment), I get that --force-exclusion will tell rubocop to ignore the files that are specified in your configuration. Is that it?

If you don't want to check a specific file, why run Flycheck on it then? Do you have a specific use case that requires this option?

@mmcnl

This comment has been minimized.

mmcnl commented Oct 25, 2017

Thanks for the reply, @fmdkdd . My use case (which I believe is common) is to be able to have the rubocop checker globally enabled in my Rails-based project without being swamped with error highlighting when I open the db/schema.rb file. (This file is continually auto-generated and thus it's impossible for me to fix the style offenses that get re-created there). This is the use case (for linters like sublime-linter or atom-linter) for which the rubocop team added the --force-exclusion option.

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Oct 26, 2017

This is the use case (for linters like sublime-linter or atom-linter) for which the rubocop team added the --force-exclusion option.

Maybe I'm missing something, but this sounds odd. Why do we need that option to achieve this? Isn't it simpler to just ask Flycheck not to lint these files, using whichever predicate you'd use to turn that option on?

IOW, what does it gain us to run rubocop on some file a.rb with the --force-exclusion a.rb flag? Why is that better than just disabling Flycheck for a.rb?

Maybe it'd help if you can show how you'd set this hypothetical new variable :) Let's assume we add flycheck-rubocop-force-exclusion, taking a list of files. How would you use it?

@cpitclaudel

This comment has been minimized.

Member

cpitclaudel commented Oct 26, 2017

Oh, wait, I think I understand now. --force-exclusion doesn't take a file name, right?
This makes a lot more sense now. @fmdkdd, the trick is that that option saves you from duplicating rubocop's exclusion logic in your Flycheck config. Instead you just feed everything to rubocop, and it returns early if you actually fed it an excluded file.
I'm in favor of adding support for that flag :) It should probably be on by default, but maybe we can start by adding the option, and then turn it on by default in a few months when more people have updated?

@mmcnl

This comment has been minimized.

mmcnl commented Oct 26, 2017

Yes, I think the flag is necessary because when the rubocop the executable is being fed an explicit list of filenames (as is often the case with linters) it by default overrides any exclusions specified in the rubocop.yml config file and always processes the file.

@fmdkdd

This comment has been minimized.

Member

fmdkdd commented Oct 26, 2017

@cpitclaudel I see that the flag is mostly harmless (because if the user is the one editing the rubocop config file, they won't be surprised that Flycheck isn't checking it).

The flag was added in 2014, so I think it should be fine to set as default, since it's a better default experience.

@Simplify

This comment has been minimized.

Member

Simplify commented Oct 26, 2017

Yeah, this should just be added after --display-cop-names. Should not be an configurable option IMHO.

Simplify added a commit that referenced this issue Oct 26, 2017

Simplify added a commit that referenced this issue Oct 26, 2017

cpitclaudel added a commit that referenced this issue Oct 26, 2017

Merge pull request #1351 from flycheck/GH-1348
Add --force-exclusion flag to rubocop command
@mmcnl

This comment has been minimized.

mmcnl commented Oct 26, 2017

Thanks all for the quick and thoughtful response to this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment