fix: used finder->exclude instead of notPath for excludedDirs #474
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bocharsky-bw
reviewed
Apr 1, 2022
| @@ -143,7 +143,7 @@ private function getConfiguredFinder(Configuration $config): Finder | |||
| $finder->in($config->getDirs()); | |||
|
|
|||
| foreach ($config->getExcludedDirs() as $exclude) { | |||
| $finder->notPath($exclude); | |||
| $finder->exclude($exclude); | |||
There was a problem hiding this comment.
Hm interesting... I see notPath() could be used for excluding directories as well: https://github.com/symfony/finder/blob/231313534dded84c7ecaa79d14bc5da4ccb69b7d/Finder.php#L274-L275
I don't understand completely why it does not work 🤔
There was a problem hiding this comment.
notPath does apply to files AND directories, while exclude only excludes directories.
So basically notPath should be used if you want to exclude EVERYTHING with a specific name.
Though, I have to say, I don't find "notPath" to be a very intuitive or self-explanatory name...
|
@bocharsky-bw Yeah, I saw the failed test too and quickly fixed it. This should be fine now? |
bocharsky-bw
approved these changes
Apr 1, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #284 excluded_dirs in config matches filenames too
tested it with my project (exclude_dir "data"). My file "base_data" is now correctly extracted, while translations inside my "data" folder are ignored as expected.
Only possible issue I see is some users expecting "exclude_dirs" to also exclude filenames, which would be a bit weird as it's been marked as bug since 3 years.
Anyway, that could be easily fixed by simply adding the values of "excluded_dirs" to "excluded_names".