8000 [Component][Finder] tests and condition: contains() used on dir by gajdaw · Pull Request #4056 · symfony/symfony · GitHub
[go: up one dir, main page]

Skip to content

[Component][Finder] tests and condition: contains() used on dir #4056

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

Merged
merged 1 commit into from
May 11, 2012
Merged

[Component][Finder] tests and condition: contains() used on dir #4056

merged 1 commit into from
May 11, 2012

Conversation

gajdaw
Copy link
Contributor
@gajdaw gajdaw commented Apr 21, 2012

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

Finder::contains() and Finder::notContains() can't be used on directories.

@@ -30,7 +30,15 @@ public function accept()
return true;
}

$content = file_get_contents($this->getRealpath());
if ($this->isDir()) {
throw new \RuntimeException(sprintf('contains() can not be called on directory "%s".', $this->getRealpath()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should not throw an exception IMO as the user cannot avoid calling it on a dir as soon as the folder in which it searches for files has subfolders.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, if the folder matches other filters of course

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you call files() then all directories are excluded and no exception is thrown. IMHO the question is: what should be done when user calls:

$finder->in('some/dir')->contains('abc');

Following your suggestion I return false.

@travisbot
Copy link

This pull request fails (merged f2fea97 into 919604a).

fabpot added a commit that referenced this pull request May 11, 2012
Commits
-------

f2fea97 [Component][Finder] tests and condition: contains() used on dir

Discussion
----------

[Component][Finder] tests and condition: contains() used on dir

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
Fixes the following tickets: -
Todo: -

`Finder::contains()` and `Finder::notContains()` can't be used on directories.

---------------------------------------------------------------------------

by travisbot at 2012-05-08T06:33:11Z

This pull request [fails](http://travis-ci.org/symfony/symfony/builds/1273818) (merged f2fea97 into 919604a).
@fabpot fabpot merged commit f2fea97 into symfony:master May 11, 2012
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
0