empty arrays should be valid globs about is-valid-glob HOT 18 CLOSED

+1

from is-valid-glob.

Yeah, let's go ahead and do this - the change to make them invalid broke a lot of stuff for people

from is-valid-glob.

sounds good to me

from is-valid-glob.

If someone wants to do a pr I'd be happy to merge. I can do it but it might be this weekend

from is-valid-glob.

We've only heard feedback from a few people so far. I'd like to hear more. There are a lot of people using gulp 4 and this is the first we are really hearing about it

from is-valid-glob.

@phated This change was made recently (the last vinyl-fs) so it's expected that we're hearing about it now

from is-valid-glob.

@contra yep, that's why I think we should leave the issues open for a little while longer. Most discussion has just happened this week and some people that might want to contribute could be super busy.

from is-valid-glob.

I'll leave the call to you guys. Just let me know what you decide.

from is-valid-glob.

also added both of you as collabs. I'll do that same on npm

from is-valid-glob.

@phated Most important to me is that we broke people's real code for no reason other than validation - we should fix it immediately and discuss afterwards, instead of leaving people's code broken while we have an open-ended discussion about it

from is-valid-glob.

fwiw, this seems like something that would only cause issues the way it is now, but probably won't cause any issues (with glob) if we revert back to allow empty arrays.

from is-valid-glob.

@contra I understand what you are saying; however, people are using alpha software that has broken many times in the past and will break many times before it ships. The reason 4.0 hasn't shipped yet is due to these "open-ended discussions" and coming up with the correct APIs. This isn't the land of 0.x, 1.x and 2.x anymore.

from is-valid-glob.

Instead of throwing inside vinyl-fs, this can be solved by switching to just a warning message. We have a real logger now and can do things like that.

from is-valid-glob.

@phated 👍 they use alpha software on their own risk, so.. no need of commenting.

But I'm 👎 for doing that on this library. There's no logic to me. Why some would want to receive something if empty array? I mean.. there have another ways to do that, for example just globstars **/*.* or something - is it so hard to someone to type few more chars? - can't got it, seriously.

For me it's not a job of library like this one to do gulp-ish things, handle these cases in your side, this is gulp/vinyl-fs job and can be handled with some if - i cant see the problem.

I'm not trolling, just thinking.

gulpjs/vinyl-fs#40 (comment) 👍

from is-valid-glob.

Cross post: I think that an invalid glob is something that will never match a file; e.g. an empty string, an empty array, an array of empty strings, etc.

So I think this is the correct behavior for this library to have. If it is determined we shouldn't throw in vinyl-fs, that doesn't change what this library should be doing.

from is-valid-glob.

based on @phated's last comment, is it safe to assume the consensus is to leave this lib as-is? It makes sense to me, since technically an empty array is an invalid glob, but I'm still happy to leave the decision to you since gulp would be most effected by any decision here.

from is-valid-glob.

@jonschlinkert I think to resolve this issue, we should leave it as is because it is truly an invalid glob. If we decide to relax the error case in gulp, that shouldn't be propagated to this library.

from is-valid-glob.

👍 sounds good, thanks

from is-valid-glob.