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] Add recursive .gitignore files support#43150

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

Conversation

@julienfalque
Copy link
Contributor

@julienfalquejulienfalque commentedSep 23, 2021•
edited
Loading

QA
Branch?5.4
Bug fix?no
New feature?yes
Deprecations?no
Tickets-
LicenseMIT
Doc PRTODO

This PR extendsFinder's.gitignore support to nested files.

Note: as a side effect, a.gitignore file is not required at the top level anymore. Actually, if no.gitignore file is found at all, no errors will be triggered.

mvorisek reacted with thumbs up emojiSpacePossum, keradus, and ging-dev reacted with rocket emoji
@carsonbot
Copy link

Hey!

I think@simPod has recently worked with this code. Maybe they can help review this?

Cheers!

Carsonbot

@fabpot
Copy link
Member

Thank you@julienfalque.

@julienfalque
Copy link
ContributorAuthor

Thanks@fabpot@nicolas-grekas@keradus!

@julienfalquejulienfalque deleted the finder-recursive-gitignore branchSeptember 28, 2021 12:27
@stof
Copy link
Member

This seems to look for.gitignore files up to the root of the flesystem, which won't play well with open_basedir restrictions. We should have a way to limit the recursion to parents.

@julienfalque
Copy link
ContributorAuthor

The look up is supposed tostop at the initial search directory, though no tests prove that. I'll try to add some.

@stof
Copy link
Member

@julienfalque is this!== working fine on Windows, or do we have issues of/ vs\ as separator ?

@julienfalque
Copy link
ContributorAuthor

Looks like this was not working fine on Windows (tests seem to cause an infinite loop on AppVeyor). See#43239.

@fabpotfabpot mentioned this pull requestNov 5, 2021
@fabpotfabpot mentioned this pull requestNov 5, 2021
fabpot added a commit that referenced this pull requestApr 1, 2022
…ulienfalque)This PR was merged into the 6.1 branch.Discussion----------[Finder] Look for gitignore patterns up to git root| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | no| New feature?  | yes-ish| Deprecations? | no| Tickets       |#43150 (comment)| License       | MIT| Doc PR        | -Commits-------744392d [Finder] Look for gitignore patterns up to git root
{
$file = $this->current();

$fileRealPath = $file->getRealPath();
Copy link
Contributor

@mvorisekmvorisekJul 6, 2023•
edited
Loading

Choose a reason for hiding this comment

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

@julienfalque realpath can be slow and I do not think this is actually correct by git spec at all

Imagine a symlink excluded by a .gitignore. The symlink will not be ignored a it is resolved here.

seehttps://stackoverflow.com/questions/40460216/how-do-i-make-git-ignore-symlink#40461160

also there should be no is_dir/is_dir, simply traverse the path up

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@fabpotfabpotfabpot approved these changes

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

+2 more reviewers

@mvorisekmvorisekmvorisek left review comments

@keraduskeraduskeradus left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Projects

None yet

Milestone

5.4

Development

Successfully merging this pull request may close these issues.

7 participants

@julienfalque@carsonbot@fabpot@stof@nicolas-grekas@mvorisek@keradus
[8]ページ先頭
©2009-2025 Movatter.jp