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

Include/Exclude Globbing with '**' Should Match Files In Base Directory #1011

Closed
jfelchner opened this issue Apr 16, 2014 · 7 comments
Closed
Assignees

Comments

@jfelchner
Copy link
Contributor

The way that ** works in unix and Ruby itself is typically that it means "match zero or more subdirectories". Unfortunately the way Rubocop handles it, it matches "one or more subdirectories".

This means that you have to have this:

Exclude:
  - 'Gemfile'
  - '**/Gemfile'
  - 'config/unicorn.rb'
  - '**/config/unicorn.rb'
  - 'db/schema.rb'
  - '**/db/schema.rb'
  - 'config/deploy.rb'
  - '**/config/deploy.rb'
  - 'config/initializers/*'
  - '**/config/initializers/*'
  - 'db/migrate/*'
  - '**/db/migrate/*'

Instead of this:

Exclude:
  - '**/Gemfile'
  - '**/config/unicorn.rb'
  - '**/db/schema.rb'
  - '**/config/deploy.rb'
  - '**/config/initializers/*'
  - '**/db/migrate/*'

In fact, you can check that this is the case by opening up an IRB session in your current project base dir and run this:

require 'pathname'
Pathname.glob(ENV['PWD'] + '/**/.rubocop.yml')

And you'll see that it does in fact return the file in the base of your project.

@jfelchner
Copy link
Contributor Author

I just noticed that you all are reinventing the wheel on a lot of the path stuff, personally I would look at the Pathname builtin library in Ruby. It does all of that stuff for you. I think it could help simplify the codebase quite a bit.

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 17, 2014

Ouch, it seems we made a serious mistake. For some reason I thought we were actually using Pathname for this. :-) @jonas054 would you have a look this?

@jonas054
Copy link
Collaborator

Absolutely. Thanks for the pointer, @jfelchner!

@jonas054 jonas054 self-assigned this Apr 17, 2014
@jonas054
Copy link
Collaborator

It's a problem that we can't use '**/config/unicorn.rb' to match config/unicorn.rb. We have, however, added a special case for bare filenames so that 'Gemfile' matches Gemfile and dir/Gemfile.

With the File.fnmatch matching that we use, 'vendor/**' (which we have in default.yml) matches for example vendor/dir/file.rb. With Pathname.glob matching, we'd have to change it to 'vendor/**/*'.

The question is, do we just switch to Pathname.glob matching, breaking backwards compatibility, or do we keep old functionality alongside the new and issue deprecation warnings?

@bbatsov
Copy link
Collaborator

bbatsov commented Apr 19, 2014

I'd suggest the deprecation notice option. At least for a couple of versions we have to warn users that the config format has changed.

@jonas054
Copy link
Collaborator

@jfelchner Could you explain in more detail what you meant when you said that we're reinventing the wheel? Because I don't quite see it. Thanks!

bbatsov added a commit that referenced this issue Apr 21, 2014
[Fix #1011] Add pattern matching with Dir#[] for config.
@jfelchner
Copy link
Contributor Author

@jonas054 thanks so much for jumping on this. The response was more than I expected. 😄 I'll try to do some code refactoring in a PR to show you what I'm referring to, but no promises on timeframe. I'm currently releasing a large refactoring of ruby-progressbar as well as a new version of chamber.

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