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

Some dynamic patterns don't work together #316

Closed
vladdu opened this issue Jun 19, 2021 · 5 comments
Closed

Some dynamic patterns don't work together #316

vladdu opened this issue Jun 19, 2021 · 5 comments
Milestone

Comments

@vladdu
Copy link

vladdu commented Jun 19, 2021

Environment

  • OS Version: Ubuntu 20.04
  • Node.js Version: 14.17

Actual behavior

Using dynamic patterns that start with ../ together with other dynamic patterns that will be put into the '.' group, doesn't work. All patterns are put inside the '.' group and my guess is that the file system walker won't go outside of the current directory and the ../ patterns will get ignored.

For example,

[
  "{a,b}/**/*.md",
  "../**/*.html",    // these get ignored
]
[
  "a/**/*.md",
  "b/**/*.md",
  "../**/*.html",    // these get ignored
]

Expected behavior

All patterns are used.

Steps to reproduce

I am using this from a third party application, so I'm not sure how to reproduce other than creating patterns similar to the above that match something on your disk.

@vladdu
Copy link
Author

vladdu commented Jun 19, 2021

I created a simple PR #317 that fixes this issue, but I'm not sure if it affects other things.

@mrmlnc
Copy link
Owner

mrmlnc commented Jun 22, 2021

At first glance, we must pick the patterns with the .. base directory to a separate group. Or it's time for us to abandon the task grouping in favor of something else.

@vladdu
Copy link
Author

vladdu commented Jun 22, 2021

The solution that minimizes the amount of file walking is to compare all patterns and see which ones include others and pick the topmost one as base for the group. So in our example, the base dir would be ../

@vladdu
Copy link
Author

vladdu commented Jun 22, 2021

Are absolute paths patterns allowed? In that case, just checking for ./ and ../ and ../ won't be enough, there can be collisions anyway, and let's not even mention symlinks. I suppose that if absolute paths are used, the user can accept the responsibility that there might be not optimally efficient (if it's documented) and it might be enough to consider only the relative patterns.

@mrmlnc
Copy link
Owner

mrmlnc commented Jun 26, 2021

Fixed by #320. Will be shipped on this week.

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

No branches or pull requests

2 participants