Skip to content

[Finder] Fix finding VCS re-included files in excluded directory #45095

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

Merged
merged 1 commit into from
Jan 26, 2022

Conversation

julienfalque
Copy link
Contributor

Q A
Branch? 5.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #45048
License MIT
Doc PR -

This PR fixes a bug where Finder would not return a file inside a directory when gitignore rules exclude the directory but re-include the file.

The first commit makes Finder return the file. Though, as shown by the second commit, the directory is still not returned. Strictly speaking, the directory is ignored, so in some sense Finder is correct. But then it contains a file that is not ignored. In this case, Git would see the directory anyway. Should Finder return any ignored directory as soon as it contains a file that is not ignored? Even if that file is not part of Finder results?

@nicolas-grekas nicolas-grekas force-pushed the gitignore-reincluded-files branch from 9a43190 to d108ac5 Compare January 26, 2022 16:34
@nicolas-grekas
Copy link
Member

Thank you @julienfalque.

@nicolas-grekas nicolas-grekas merged commit 79d1101 into symfony:5.4 Jan 26, 2022
@julienfalque julienfalque deleted the gitignore-reincluded-files branch January 26, 2022 16:42
@julienfalque
Copy link
Contributor Author

Thanks for merging @nicolas-grekas! What's your opinion about returning the directory as well (see #45095 (comment))?

@nicolas-grekas
Copy link
Member

I don't know, I'm not into that business :)
Someone that has the use case should tell.

This was referenced Jan 28, 2022
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.

3 participants