Skip to content

[Finder] Ignore paths from .gitignore #26714 #30448

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
Merged
merged 1 commit into from
Mar 22, 2019
Merged

[Finder] Ignore paths from .gitignore #26714 #30448

merged 1 commit into from
Mar 22, 2019

Conversation

ahmedsbytes
Copy link

@ahmedsbytes ahmedsbytes commented Mar 5, 2019 •
edited
Loading

Q A
Branch? master for features
Bug fix? no
New feature? yes
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #26714
License MIT
Doc PR symfony/symfony-docs#...

Implementation of feature request #26714

Finder::ignoreVCS() is great at ignoring file patterns for the files created by popular VCS systems.

However, it would be great to be able to instruct Finder to actually exclude the paths excluded by .gitignore.

So if we have .gitignore:

vendor/
cache/

Finder::create()
->files()
->ignoreVCS(true) // <--- Ignores .git
->ignoreVCSIgnored(true); // <--- Ignores vendor/ and cache/

@ahmedsbytes ahmedsbytes changed the title issue #26714 v1 [Finder] Ignore paths from .gitignore #26714 Mar 5, 2019
@ahmedsbytes ahmedsbytes force-pushed the feature/issue_26714 branch from 54c6b04 to 2bfe1de Compare March 5, 2019 01:22
derrabus
derrabus requested changes Mar 5, 2019
View reviewed changes
@nicolas-grekas nicolas-grekas added this to the next milestone Mar 7, 2019
@dimabory
Copy link
Contributor

dimabory commented Mar 7, 2019 •
edited
Loading

@amaabdou could you please take a look at this https://labs.consol.de/development/git/2017/02/22/gitignore.html and add more test with advanced use cases?

@ahmedsbytes
Copy link
Author

I added as many cases I could,If I missed anything or more cases need or something is unclear
Or more reviews needed
please add a comment

@ahmedsbytes
Copy link
Author

ahmedsbytes commented Mar 12, 2019 •
edited
Loading

also I could not change PR from Draft to ready to be reviewed, I do not see the "Ready to review" button appeared today not sure how :D

@ahmedsbytes ahmedsbytes marked this pull request as ready for review March 12, 2019 08:53
@ahmedsbytes
Copy link
Author

is there anything I can do to move this forward ?

OskarStark
OskarStark approved these changes Mar 17, 2019
View reviewed changes
Copy link
Contributor

@OskarStark OskarStark left a comment

Choose a reason for hiding this comment

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

Great job 🔥

fabpot
fabpot requested changes Mar 19, 2019
View reviewed changes
@ahmedsbytes
Copy link
Author

is there anything else I can do to move this forward ?

fabpot
fabpot approved these changes Mar 22, 2019
View reviewed changes
Copy link
Member

@fabpot fabpot left a comment

Choose a reason for hiding this comment

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

Can you add a note in the component CHANGELOG file?

@ahmedsbytes
Copy link
Author

@fabpot just did , is this one okay ?

@fabpot fabpot force-pushed the feature/issue_26714 branch from f7a664e to 9491393 Compare March 22, 2019 07:19
@fabpot
Copy link
Member

fabpot commented Mar 22, 2019

Great work @amaabdou, thank you

@fabpot fabpot merged commit 9491393 into symfony:master Mar 22, 2019
fabpot added a commit that referenced this pull request Mar 22, 2019
@fabpot
feature #30448 [Finder] Ignore paths from .gitignore #26714 (amaabdou)
0b2a9d5 
This PR was squashed before being merged into the 4.3-dev branch (closes #30448).

Discussion
----------

 [Finder] Ignore paths from .gitignore #26714

| Q             | A
| ------------- | ---
| Branch?       | master for features
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #26714
| License       | MIT
| Doc PR        | symfony/symfony-docs#... <!-- required for new features -->

<!--
Write a short README entry for your feature/bugfix here (replace this comment block.)
This will help people understand your PR and can be used as a start of the Doc PR.
Additionally:
 - Bug fixes must be submitted against the lowest branch where they apply
   (lowest branches are regularly merged to upper ones so they get the fixes too).
 - Features and deprecations must be submitted against the master branch.
-->
Implementation of feature request #26714

Finder::ignoreVCS() is great at ignoring file patterns for the files created by popular VCS systems.

However, it would be great to be able to instruct Finder to actually exclude the paths excluded by .gitignore.

So if we have .gitignore:

vendor/
cache/

Finder::create()
	->files()
    ->ignoreVCS(true) // <--- Ignores `.git`
	->ignoreVCSIgnored(true); // <--- Ignores vendor/ and cache/

Commits
-------

9491393  [Finder] Ignore paths from .gitignore #26714
@OskarStark
Copy link
Contributor

Thank you for this great new feature @amaabdou, would you please create a documentation PR in the symfony-docs repository?

@SpacePossum SpacePossum mentioned this pull request Mar 22, 2019
Closed
@SpacePossum
Copy link
Contributor

thanks @amaabdou :)

@ahmedsbytes ahmedsbytes deleted the feature/issue_26714 branch March 22, 2019 10:22
@ahmedsbytes
Copy link
Author

@OskarStark Yep yep, of course, working on this

@hkdobrev
Copy link
Contributor

Thank you @amaabdou! ❤️

@ahmedsbytes ahmedsbytes mentioned this pull request Mar 25, 2019
Merged
javiereguiluz added a commit to symfony/symfony-docs that referenced this pull request Mar 26, 2019
@javiereguiluz
minor #11224 Adding ignoreVCSIgnored documentation (amaabdou)
bd1697e 
This PR was merged into the master branch.

Discussion
----------

Adding ignoreVCSIgnored documentation

<!--

If your pull request fixes a BUG, use the oldest maintained branch that contains
the bug (see https://symfony.com/roadmap for the list of maintained branches).

If your pull request documents a NEW FEATURE, use the same Symfony branch where
the feature was introduced (and `master` for features of unreleased versions).

-->
Adding documentation for symfony/symfony#30448

Commits
-------

6d7ec3f adding ignoreVCSIgnored documentation
@stof
Copy link
Member

stof commented Mar 27, 2019

ignoreVCS handles all VCS (well, at least all the ones we know about).
This new ignoreVCSIgnored seems to be actually ignoreGitIgnored as it only cares about .gitignore.

I think we should either add support for other VCS or rename method.

And this method is also broken, as it looks for a .gitignore file in the directory passed to in(). But that file may be present in a parent folder (a Symfony project often has all gitignore rules defined at the project root, even when ignoring nested files/folders) or in subdirectories (with rules applying only in the given subdirectory).

This was referenced Mar 28, 2019
Closed
Closed
@ahmedsbytes
Copy link
Author

ahmedsbytes commented Mar 28, 2019 •
edited
Loading

@stof You are right, I've created two issues

one for the parent directory search, #30739.
another one for adding support for mercurial #30738, not sure what other version control systems also to suggest to add

Hopefully will work on them on weekend if no one else worked on them

@nicolas-grekas nicolas-grekas modified the milestones: next, 4.3 Apr 30, 2019
@fabpot fabpot mentioned this pull request May 9, 2019
Merged
@soullivaneuh soullivaneuh mentioned this pull request Mar 19, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ro0NL ro0NL ro0NL left review comments

@derrabus derrabus derrabus requested changes

@fabpot fabpot fabpot approved these changes

@OskarStark OskarStark OskarStark approved these changes

Assignees
No one assigned
Labels
Feature Status: Reviewed
Projects
None yet
Milestone
4.3
Development

Successfully merging this pull request may close these issues.