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

When multiple files are matched, it includes both of them. #93

Open
skeddles opened this issue Feb 22, 2019 · 7 comments
Open

When multiple files are matched, it includes both of them. #93

skeddles opened this issue Feb 22, 2019 · 7 comments
Labels

Comments

@skeddles
Copy link

I know this is abandoned, just thought I'd document the issue.

If you specify multiple paths with includePaths in the options, it will search all paths for your file, and include all the matches, rather than just the first one.

@ryios
Copy link

ryios commented Mar 2, 2019

It's a shame it's abandoned, it's a good plugin. I use it a lot.

@KenEucker
Copy link
Collaborator

@skeddles, @ryios UPDATE:

I was able to have a conversation with the maintainer, @wiledal, and have agreed to provisionally join the github project to address the open pull requests. I am going to take some time to respond to the ones that seem the most important but I also urge anyone to make comments on any items that they feel are pertinent.

After I've approved and merged a few of the more important PRs (dependency updates), I will ask the community to pull down the master branch and test it before we publish a new version to NPM.

Thank you for your patience and understanding!

@wiledal wiledal added the bug label Apr 6, 2019
@KenEucker
Copy link
Collaborator

KenEucker commented Apr 7, 2019

@skeddles, I can see a use case for this being the intended behavior of the plugin. There may even be projects out there which are currently taking advantage of this behavior. For this reason, I worry that changing the behavior would negatively impact the community.

To provide a solution to this problem we could implement a param which tells the plugin to stop inserting a file after the first match, and name that param something like includeOnce which would default to false.

Thoughts?

@skeddles
Copy link
Author

skeddles commented Apr 7, 2019

I think stopping at one would be a better default behavior, but I don't really know how changing it would affect others so if you have to make including all default that's fine.

includeOnce seems more like it's talking about include vs require, like only include the same file once if it's there multiple times. Maybe something like ignoreDuplicateMatches would be clearer.

@KenEucker
Copy link
Collaborator

@skeddles, I like ignoreDuplicateMatches, thank you for that suggestion. Did you happen to come up with a solution for this, or does this no longer apply to your work? If not, I can come up with a solution for this shortly if this is still affecting you.

The use case I am thinking of regarding the existing behavior might actually be an edge case, so we could instead go with a param that instructs the opposite: includeDuplicateMatches.

I use the nomenclature "include" because it matches the existing convention in the source code. The plugin name is gulp-include, but I don't think that is a direct reference to the include keyword. Internally the code uses params like includePaths and references all files as includedFiles, which means all files being processed by the plugin. I readily recognize your confusion and think it would be good to reduce that confusion, but that's also what the documentation is for!

@skeddles
Copy link
Author

skeddles commented Apr 8, 2019

I ended up just prefixing my filenames, so I don't need it currently, but would great to have.

@ghost
Copy link

ghost commented Jul 2, 2019

@skeddles, I can see a use case for this being the intended behavior of the plugin. There may even be projects out there which are currently taking advantage of this behavior. For this reason, I worry that changing the behavior would negatively impact the community.

Yes, we use this behavior as a feature.

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

No branches or pull requests

4 participants