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

[Finder] ExcludeDirectoryFilterIterator does not like trailing slashes #19599

Closed
kassner opened this issue Aug 11, 2016 · 4 comments
Closed

[Finder] ExcludeDirectoryFilterIterator does not like trailing slashes #19599

kassner opened this issue Aug 11, 2016 · 4 comments

Comments

@kassner
Copy link

kassner commented Aug 11, 2016

Hi!

With PHPCPD (sebastianbergmann/phpcpd#135) I found out that ExcludeDirectoryFilterIterator does not exclude a directory if it is informed with a trailing slash (very common to happen if you're using bash autocomplete).

Looks like just rtrim($directory, '/') here will do the trick, but I was not able to assemble a unit test yet.

Any other approach to fix this?

Thanks!

@ro0NL
Copy link
Contributor

ro0NL commented Aug 13, 2016

Can you show what $directories looks like in your case? I think this makes sense :-)

@kassner
Copy link
Author

kassner commented Aug 13, 2016

@ro0NL

array(10) {
  [0]=>
  string(6) "tests/"
  [1]=>
  string(4) ".svn"
  [2]=>
  string(4) "_svn"
  [3]=>
  string(3) "CVS"
  [4]=>
  string(6) "_darcs"
  [5]=>
  string(12) ".arch-params"
  [6]=>
  string(9) ".monotone"
  [7]=>
  string(4) ".bzr"
  [8]=>
  string(4) ".git"
  [9]=>
  string(3) ".hg"
}

@ro0NL
Copy link
Contributor

ro0NL commented Aug 13, 2016

Here's the test 2.7...ro0NL:finder/rtrim-excluded-dirs

I can PR this if you want.. but maybe you have something going on already. The class is complex though.. i cant really visualize it how it works in recursive state (ie ::$isRecursive) as this is untested :-) (ie maybe it's only needed for the if part).

@kassner
Copy link
Author

kassner commented Aug 13, 2016

@ro0NL you can PR it, I'm not that far with my test.

fabpot added a commit that referenced this issue Sep 28, 2016
…FilterIterator (ro0NL)

This PR was merged into the 2.7 branch.

Discussion
----------

[Finder] Trim trailing directory slash in ExcludeDirectoryFilterIterator

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19599
| License       | MIT
| Doc PR        | reference to the documentation PR, if any

In this context `path` equals `path/`

Commits
-------

e0e5f0c apply rtrim
@fabpot fabpot closed this as completed Sep 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants