Accept glob string for [entry files] ...
...in command "browserify [entry files] {OPTIONS}" Windows does not auto expand globs on the command line
Creating the pull request to try and progress. It works locally for me, but an automated test case would be awesome. I can't tell how to make such a test case examining the existing suite.
This pull request is for https://github.com/substack/node-browserify/issues/1170
I'm reviewing
I think something similar the following would be prefered:
[].concat(opts.entries).filter(Boolean).reduce(function (globbed, input) {
return globbed.concat(glob.sync(input));
}, []).forEach(function (file) {
self.add(file, { basedir: opts.basedir });
});
I'd also prefer to isolate globs to being expanded in "bin/args.js", rather than in the library code.
Ah this would work great in watchify (see https://github.com/substack/watchify/issues/56). Don't forget to test it with the CLI not just the API :stuck_out_tongue:
@terinjokes Yeah, I see how that fits more in with the current pattern. I just need to spend some more time to understand the current pattern a bit better. I'll try to at least get around copy pasting your suggested change and testing it.
@zertosh That issue does not appear to be the same thing. That issue is about NEW files. This issue is about globs simply not working at all (on Windows). Solving this problem in browserify did seem to solve the duplicate problem of globs not working with watchify when I pointed it at my edited repo.
@chad-autry My is to use the fact that entries maybe be globs to monitor for new files when implementing it in watchify - that's all.
I don't know if this should be browserify's responsibility in the API. People can always call glob() themselves here.
Adding a glob.sync() call seems bad.
@zertosh You wouldn't be able to since Linux systems would still expand the glob before it reached watchify
@substack I'll try to re-write it like the other other uses of glob in bin/args.js to not do glob.sync() For my use case I'm calling browserify from an npm script, was unable to call another program to substitute command parameters.
@chad-autry Not if it's quoted.
The use case I see in watchify is to monitor a directory of tests - where each test file needs to be an entry.
Watchify would look at b._options.entries to see if anything passes glob.hasMagic. If that's true, then watchify would setup another watcher with that glob, and on add/remove, it'll simply teardown the existing browserify instance and create a new one. This would only work with the CLI, since you have control there to create / destroy browserify instances.
Though, now that I think about it some more, this seems like a bad idea since it'll leave API users out in the cold.
I've made the logic more consistent with how browserify does things, and moved it over into "bin/args.js" as @terinjokes suggested.
What is the status of this PR ? Can it be merged ?
Requires re-review and hopefully acceptance by someone with permission to merge.
@chad-autry Thank you so much for this patch! You saved my day :) I can't believe this isn't merged yet. That is just poor. Probably going to use Webpack for further projects...
Let's have the best globally moving projects.
Please consider this improvement. It's very useful to run tests.
browserify 'test/**/*.spec.js' | tape-run