Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Finder] document array use for locations#6917

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
xabbuh merged 1 commit intosymfony:2.7frommickaelandrieu:patch-5
Sep 22, 2016

Conversation

@mickaelandrieu
Copy link
Contributor

@mickaelandrieumickaelandrieu commentedAug 25, 2016
edited
Loading

Hi,

This pull request is valid from Symfony 2.7 to 3.2-dev.

The functionFinder::in() accepts a string or array of string for look into multiple locations =>https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Finder/Finder.php

The other way described is valid, but shouldn't be recommended/documented.

Mickaël


$finder->in(__DIR__);

Search in several locations by chaining calls to
Copy link
Member

@wouterjwouterjAug 25, 2016
edited
Loading

Choose a reason for hiding this comment

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

this no longer holds true.

I propose to merge the code examples (this one and the ones on line 80 & 89):

The location is the only mandatory criteria. It tells the finder whichdirectory to use for the search::    $finder->in(__DIR__);    // search in several locations    $finder->in(array(__DIR__, '/elsewhere'));    $finder->in(__DIR__)->in('/elsewhere');    // use wildcard characters (it has to resolve to at least one directory path)    $finder->in('src/Symfony/*/*/Resources');Exclude directories from matching with the [...]

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

Hmm, it's less readable...

Is it ok for you to suggest the 2 options ? See my update :)

Copy link
Member

Choose a reason for hiding this comment

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

👍 for Wouter's suggestion.

@mickaelandrieu we're including more code, with short comments to explain them, as we've found that people mostly scan to code-blocks anyways. This is a situation where this isn't ahuge win, because the text between the paragraphs is so short, but I still like.

Copy link
Member

Choose a reason for hiding this comment

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

Except for one change:

// search inside *both* directories$finder->in(array(__DIR__,'/elsewhere'));// same as above$finder->in(__DIR__)->in('/elsewhere');

This is to be clear what callingin() twice does - it does not replace the previous directory.

@wouterj
Copy link
Member

Thanks ,makes sense. We should still keep the reference to chaining though (e.g. when dynamically adding multiple directories), see my proposal.

Status: needs work

@stof
Copy link
Member

multiple calls to->in() are handy when you make these calls conditionally.

Thus, it is good to explain to the user what happens when calling it twice (some people might expect that it replaces the previous value entirely)

@weaverryan
Copy link
Member

👍 for Wouter's version of this

Status: Reviewed

@xabbuhxabbuh merged commitbff0ce6 intosymfony:2.7Sep 22, 2016
xabbuh added a commit that referenced this pull requestSep 22, 2016
This PR was merged into the 2.7 branch.Discussion----------[Finder] document array use for locationsHi,This pull request is valid from Symfony 2.7 to 3.2-dev.The function ``Finder::in()`` accepts a string or array of string for look into multiple locations =>https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Finder/Finder.phpThe other way described is valid, but shouldn't be recommended/documented.MickaëlCommits-------bff0ce6 [Finder] document array use for locations
xabbuh added a commit that referenced this pull requestSep 22, 2016
@xabbuh
Copy link
Member

Thank you@mickaelandrieu. This is now merged. I also made a little tweak suggested by@weaverryan in232ebc3.

@mickaelandrieumickaelandrieu deleted the patch-5 branchSeptember 22, 2016 09:34
xabbuh added a commit that referenced this pull requestSep 22, 2016
* 2.7:  Several typo fixes  Update console.rst  [#6917] some tweaks after review  [Finder] document array use for locations
xabbuh added a commit that referenced this pull requestSep 22, 2016
* 2.8:  Several typo fixes  Several typo fixes  Update console.rst  [#6917] some tweaks after review  [Finder] document array use for locations
xabbuh added a commit that referenced this pull requestSep 22, 2016
* 3.1:  Several typo fixes  Several typo fixes  Several typo fixes  Update console.rst  [#6917] some tweaks after review  [Finder] document array use for locations
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@weaverryanweaverryanweaverryan left review comments

Assignees

No one assigned

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@mickaelandrieu@wouterj@stof@weaverryan@xabbuh@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp