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::path() method matching directories and files #6652

Closed
wants to merge 1 commit into from

Conversation

soyuka
Copy link
Contributor

@soyuka soyuka commented Jun 14, 2016

For clarity, adds an example where path() is used to match a directory or a file.

ref api-platform/core#575 (comment)

@@ -222,6 +222,11 @@ and adding delimiters:
The :method:`Symfony\\Component\\Finder\\Finder::notPath` method excludes files by path::

$finder->notPath('other/dir');

The method :method:`Symfony\\Component\\Finder\\Finder::path` will match both directories and files. For example, the following will match ``foo/bar.yml`` but also ``foo.yml``::
Copy link
Member

Choose a reason for hiding this comment

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

will match both directories and files -> matches both directories and files

@@ -223,6 +223,11 @@ The :method:`Symfony\\Component\\Finder\\Finder::notPath` method excludes files

$finder->notPath('other/dir');

The method :method:`Symfony\\Component\\Finder\\Finder::path` matches both directories and files. For example, the following will match ``foo/bar.yml`` but also ``foo.yml``::
Copy link
Member

Choose a reason for hiding this comment

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

I would move the word "method" after the method name.

Copy link
Member

Choose a reason for hiding this comment

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

And can you please wrap the paragraph after the first line that crosses the 72nd character?

@wouterj
Copy link
Member

wouterj commented Jun 19, 2016

Hi @soyuka! What about changing the code example on line 205 with something like this instead:

-     $finder->path('some/special/dir');
+     // matches files in the data/ directory and its subdirectories.
+     $finder->path('data');

I don't think we should add a whole new paragraph and code example for such information. It can perfectly be included in the code example imo. However, I would like to hear your and others opinions about it. If you agree, can you please update the PR?

Status: needs work

@soyuka
Copy link
Contributor Author

soyuka commented Jun 19, 2016

My point is that, path method will match the directory containing files too.

Saying:

matches files in the data/ directory and its subdirectories.

isn't sufficient imo, because it'll also match a data file. Or when filtered by name it'll match the file if it exists or sub directories.

@javiereguiluz
Copy link
Member

javiereguiluz commented Jun 19, 2016

I agree with @wouterj but I also understand what @soyuka says. This is really tricky. What about this proposal:

// matches files that contain "data" anywhere in their paths (including filenames)
$finder->path('data');

@soyuka
Copy link
Contributor Author

soyuka commented Jun 19, 2016

@javiereguiluz +1, but may we add an example that combines path and name? I think that adding name helps to understand the usage finality:

// matches files that contain "data" anywhere in their paths (files or directories)
$finder->path('data');
// for example this will match data/*.xml and data.xml if they exists
$finder->path('data')->name('*.xml');

@javiereguiluz
Copy link
Member

@soyuka +1 I like your proposal because to me this looks impossible to explain in just 1 sentence.

@wouterj
Copy link
Member

wouterj commented Jun 19, 2016

Actually, after reading your proposal, it's the first time I understand the problem this PR tries to solve. 👍 to update the example with that.

Can you please update your PR?

@soyuka
Copy link
Contributor Author

soyuka commented Jun 19, 2016

Github does weird things sorry about that, updated with my last example l205 and removed the previous paragraph.

// matches files that contain "data" anywhere in their paths (files or directories)
$finder->path('data');
// for example this will match data/*.xml and data.xml if they exist
$finder->path('data')->name('*.xml');
Copy link
Member

Choose a reason for hiding this comment

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

please indent the code example with 4 spaces. (if I count correctly, it's 2 spaces at the moment).

@xabbuh
Copy link
Member

xabbuh commented Jun 20, 2016

👍

Status: Reviewed

@javiereguiluz
Copy link
Member

👍

@xabbuh
Copy link
Member

xabbuh commented Jun 20, 2016

Thank you @soyuka.

xabbuh added a commit that referenced this pull request Jun 20, 2016
…oyuka)

This PR was submitted for the master branch but it was merged into the 2.7 branch instead (closes #6652).

Discussion
----------

`Finder::path()` method matching directories and files

For clarity, adds an example where `path()` is used to match a directory or a file.

ref api-platform/core#575 (comment)

Commits
-------

cd28675 Improve `Finder::path` code example about resulting matches
@xabbuh xabbuh closed this Jun 20, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants