Skip to content

Conversation

@shnhrrsn
Copy link
Contributor

This PR addresses the issue raised in #1283, where a single includes glob pattern with no matches causes the isIncludedPath function to ignore the includes restriction.

The problem occurs when the isIncludedPath function checks if includePaths is empty, which is true if there are no include patterns or if there are no matches for a pattern. To fix this issue, I changed includePaths to an optional, using nil to indicate that all paths should be allowed.

  • Update the isIncludedPath function to handle optional includePaths.
  • Modify the getSourceChildren function to accept an optional includePaths parameter.
  • Adjust the getSourceFiles function to work with optional includePaths.
  • Update the generate function to handle an optional includePaths.

Additionally, I added a new test case to verify the correct behavior when no matches are found for an includes pattern.

Copy link
Owner

@yonaskolb yonaskolb left a comment

Choose a reason for hiding this comment

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

Thanks for investigating and fixing this @shnhrrsn. Could you also please add a changelog entry

return !defaultExcludedFiles.contains(where: { path.lastComponent == $0 })
&& !(path.extension.map(defaultExcludedExtensions.contains) ?? false)
&& !excludePaths.contains(path)
// If includes is empty, it's included. If it's not empty, the path either needs to match exactly, or it needs to be a direct parent of an included path.
Copy link
Owner

Choose a reason for hiding this comment

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

Is this comment still accurate, or does it need changing?

@yonaskolb
Copy link
Owner

Thanks @shnhrrsn!

@yonaskolb yonaskolb merged commit 3a7e75f into yonaskolb:master Aug 17, 2023
@shnhrrsn shnhrrsn deleted the sh/includes-fix branch August 17, 2023 04:32
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.

2 participants