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

fix srcDir + filtering #135

Merged
merged 1 commit into from
Apr 13, 2021
Merged

fix srcDir + filtering #135

merged 1 commit into from
Apr 13, 2021

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Mar 30, 2021

PR #120 broke the case when you have both srcDir and some filtering options like exclude. The included test demonstrates the bug. It crashes during processFile because the sourcePath doesn't exist.

This PR fixes the bug by restoring the correct relative paths as inputs to walkSync.

PR broccolijs#120 broke the case when you have both `srcDir` and some filtering options like `exclude`. The included test demonstrates the bug. It crashes during `processFile` because the `sourcePath` doesn't exist.

This PR fixes the bug by restoring the correct relative paths as inputs to `walkSync`.
ef4 added a commit to ef4/broccoli-funnel that referenced this pull request Mar 30, 2021
 - I need the newer walk sync because it handles symlink cycles
 - there's a newer broccoli-funnel that already updated, but that one is full of bugs (broccolijs#135 broccolijs#136) and I don't have time right now to unfuck it.
@k3n
Copy link

k3n commented Apr 13, 2021

I have a similar issue from using srcDir, destDir, and include.

I tested your changes and they resolve the problem.

@rwjblue Can you review please?

@rwjblue rwjblue added the bug label Apr 13, 2021
@rwjblue rwjblue merged commit 093011e into broccolijs:master Apr 13, 2021
@k3n
Copy link

k3n commented Apr 13, 2021

Wow, fast! Appreciated.

@ef4
Copy link
Contributor Author

ef4 commented Apr 14, 2021

There are other regressions that remain unfixed that stem from the same changes that introduced this bug, see #136

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants