Skip to content

Conversation

@blelump
Copy link

@blelump blelump commented Aug 17, 2017

When Exclude param is used for a cop, ie:

Metrics/BlockLength:
  Exclude:
    - 'spec/**/*.rb'

Rubocop internally checks these strings using the match_pattern? method
and since the pattern arg is given as project root + pattern, it never
works for relative paths. It would, however, work for a path defined as
regexp, ie:

Metrics/BlockLength:
  Exclude:
    - !ruby/regexp /spec\/.*\/.*_spec.rb$/

however I think it's convenient to support string paths in order
to allow '**' pattern as well.

When Exclude param is used for a cop, ie:

```
Metrics/BlockLength:
  Exclude:
    - 'spec/**/*.rb'
```

Rubocop internally checks these strings using the [`match_pattern?`][1] method
and since the pattern arg is given as `project root + pattern`, it never
works for relative paths. It would, however, work for a path defined as
regexp, ie:

```
Metrics/BlockLength:
  Exclude:
    - !ruby/regexp /spec\/.*\/.*_spec.rb$/
```

however I think it's convenient to support string paths in order
to allow '**' pattern as well.

[1]: https://github.com/bbatsov/rubocop/blob/v0.47.1/lib/rubocop/path_util.rb#L17
@madslundholmdk
Copy link

@blelump what is the status of this?

@blelump
Copy link
Author

blelump commented Feb 9, 2018

@madslundholmdk , what do you mean?

@madslundholmdk
Copy link

@blelump why isn't this merged yet?

@blelump
Copy link
Author

blelump commented Feb 9, 2018

@madslundholmdk , this is just my PR for a particular problem :-) . Whether the PR will be merged or not eventually , depends on the repo owner (which I am not).

@madslundholmdk
Copy link

@m4i can this be merged? I just tested it, and it fix my issue #38

@tiagoefmoraes
Copy link

@m4i this change worked for me too. Do you need any help to merge this?

For now I'm using specific_install to use this fix:
gem specific_install -l [email protected]:blelump/rubocop-git.git -r 79a6579b7ebbd82890c32f536fe78472fe816071

@tfwright
Copy link

I just switched my config to use the regex from the OP...not sure what downsides might be though 🤷‍♂

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

Successfully merging this pull request may close these issues.

4 participants