Skip to content

[Finder] ExcludeDirectoryFilterIterator does not like trailing slashes #19599

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.

Sign up for GitHub

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

Jump to bottom
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
Labels
Bug Finder Status: Needs Review

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 •
edited
Loading

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 •
edited
Loading

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.

@ro0NL ro0NL mentioned this issue Sep 16, 2016
Merged
fabpot added a commit that referenced this issue Sep 28, 2016
@fabpot
bug #19951 [Finder] Trim trailing directory slash in ExcludeDirectory…
40c2cbd 
…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
Assignees
No one assigned
Labels
Bug Finder Status: Needs Review
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @javiereguiluz @kassner @ro0NL @xabbuh @carsonbot