Skip to content
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

Include excluded files #2593

Merged
merged 7 commits into from Mar 14, 2024
Merged

Conversation

mamazu
Copy link
Contributor

@mamazu mamazu commented Mar 9, 2024

Fixes #2485

The include and exclude logic now allows for included paths inside excluded paths.
Also the tests for the file provider now also check that it returns the correct files and not just the count.

How this worked before

Call includes to filter the list of files to include and then call exclude to remove files from that list that are excluded.

How it works now

Build a map of paths and if they are included or not and tries to match all files against that map to see if they should be included or not.

@mamazu mamazu force-pushed the phpactor-2485-include-exclude branch from ad66886 to be82766 Compare March 9, 2024 14:22
@mamazu mamazu force-pushed the phpactor-2485-include-exclude branch from 5996291 to abff473 Compare March 9, 2024 15:37
Copy link
Collaborator

@dantleech dantleech left a comment

Choose a reason for hiding this comment

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

Looks OK, left a couple of minor comments 👍

lib/Filesystem/Domain/FileList.php Outdated Show resolved Hide resolved
if($compare !== 0) {
return $compare;
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

can you add a commenta about what this is doing? i.e. if the segments are the same length then... ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What exactly should I comment here. If they are the same then their order doesn't matter.

Copy link
Collaborator

@dantleech dantleech Mar 14, 2024

Choose a reason for hiding this comment

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

explaining what the different branches mean. It's not obvious what ==='** and ==='*' mean and why we return. what happens with baz* or *baz or with */baz/**/boo*?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yeah wildcards within a path aren't handled. That's a valid point.

lib/Filesystem/Domain/FileList.php Outdated Show resolved Hide resolved
lib/Filesystem/Domain/FileList.php Show resolved Hide resolved
$partsB = explode(DIRECTORY_SEPARATOR, $b);
$countDiff = count($partsA) <=> count($partsB);
if ($countDiff !== 0) {
// Longer paths should come first
Copy link
Collaborator

Choose a reason for hiding this comment

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

paths with more segments.

@dantleech dantleech merged commit ea02ce0 into phpactor:master Mar 14, 2024
10 checks passed
@mamazu mamazu deleted the phpactor-2485-include-exclude branch March 24, 2024 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Include and exclude patterns
2 participants