10BC0 [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

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.

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 4737

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

0