gulpjs / glob-stream

Readable streamx interface over anymatch.
MIT License
178 stars 53 forks source link

Emit error on file not found with singular globs #41

Closed callumacrae closed 9 years ago

callumacrae commented 9 years ago

gulpjs/gulp#374

coveralls commented 9 years ago

Coverage Status

Coverage decreased (-1.25%) to 98.75% when pulling 4eacfeb0f3939de6e612a19b7e8a1a385b19a61b on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 7c321c5e4aff378507e8b7f81267caae6f7d22f0 on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 0216b076a633c8f0249492ceb74ae67d8c1c13e1 on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

yocontra commented 9 years ago

I think this PR was based on the assumption that it should only fail if one direct path is passed in, but it should also fail for

gulp.src(['a.js', 'b.js'])

if either a or b are not found

and also

gulp.src(['a.js', 'b*.js'])

if a.js is not found

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling e13964941794b5a4a0efa5c41de6760979535804 on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling f9e7bd802097d7898b7b2be6e1f06a3da30ba9ea on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

callumacrae commented 9 years ago

I think this PR was based on the assumption that it should only fail if one direct path is passed in, but it should also fail for

Would that behaviour be as intuitive?

It's also waay trickier to implement. I can't figure out how I would do it, because the match event doesn't say which glob was matched.

yocontra commented 9 years ago

@callumacrae Any direct path should fail if it wasn't found, only doing so when one is specified seems like the unintuitive route

yocontra commented 9 years ago

Also you do have access to the glob in match events https://github.com/callumacrae/glob-stream/blob/empty-error/index.js#L39 ourGlob is within scope

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 006467faf667c56df38671b783812abc7c5ad22f on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

callumacrae commented 9 years ago

Done. Not sure if the weird error emitting stuff I'm doing is the best way, but I don't know any other.

I got confused by the whole create / createStream thing, I thought it would be trickier.

yocontra commented 9 years ago

Looks good, just need to tweak the error management and we are :+1:

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling e7777928c8d3974e3ce6ace37a99d82b99faa734 on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

coveralls commented 9 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling e7777928c8d3974e3ce6ace37a99d82b99faa734 on callumacrae:empty-error into 6411ea4891d24227f94f302bd52d79584af8fa9b on wearefractal:master.

armandabric commented 9 years ago

Nice PR :+1:

phated commented 9 years ago

@contra this really should have been a 5.0 bump, not a 4.1

jameswomack commented 9 years ago

@phated Agreed. This changed build behavior here at Netflix.

callumacrae commented 9 years ago

Is it worth reverting it for 4.1.1 and releasing it in 5.0?

phated commented 9 years ago

@callumacrae i think so

yocontra commented 9 years ago

@phated typo'd the np command, you have access to the repo and pkg - would you mind doing the revert and republish? on a plane right now and i can barely load this webpage let alone npm publish

phated commented 9 years ago

@contra I have publish rights but not access to this repo

yocontra commented 9 years ago

Okay published as 5.0, which means now vinyl-fs needs to be bumped to 5.0 if it wants to depend on this and then gulp needs to be updated to depend on the new vinyl-fs

phated commented 9 years ago

@contra were you able to publish 4.1.1 that reverts the breaking change?

callumacrae commented 9 years ago

npm ERR! version not found: glob-stream@4.1.1

Doesn't look like it.

git checkout fac7fa8957867e7b1348261118b18185b09e5218^
git checkout -b 4.x
np
npm dist-tag add glob-stream@5.0.0 latest

should do the trick. You don't need to checkout a new branch if you don't use np.

yocontra commented 9 years ago

np totally fails when doing this, going to have to do it manually

yocontra commented 9 years ago

Also npm unpublish is totally failing. np just published a 5.1.0 with 4.x code and now I can't unpublish it. going to have to contact npm support because everything is fucked in this toolchain

callumacrae commented 9 years ago

You can't unpublish. Did you try what I said or did you start from 5.0? Because you can't do that.

yocontra commented 9 years ago

Okay all done. 4.1.1 === 4.0.1, 5.0.0 = new stuff, tagged as latest. For future reference adding --force to the end of anything makes npm work

callumacrae commented 9 years ago

:+1:

adamreisnz commented 8 years ago

Would it be possible to display the glob/filename that is causing the error? Right now it's very undescriptive and I have no idea which glob is causing the issue:

Error: File not found with singular glob
  at Glob.<anonymous> (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob-stream/index.js:34:30)
  at emitOne (events.js:77:13)
  at Glob.emit (events.js:169:7)
  at Glob._finish (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob/glob.js:172:8)
  at done (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob/glob.js:159:12)
  at Glob._processSimple2 (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob/glob.js:652:12)
  at /Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob/glob.js:640:10
  at Glob._stat2 (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob/glob.js:736:12)
  at lstatcb_ (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/glob/glob.js:728:12)
  at RES (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/inflight/inflight.js:23:14)
  at f (/Users/Adam/Sites/meanie/boilerplate/src/node_modules/once/once.js:17:25)
  at FSReqWrap.oncomplete (fs.js:82:15)
phated commented 8 years ago

This is master but we aren't able to cut a release yet.