-
-
Notifications
You must be signed in to change notification settings - Fork 17
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
first pass #2
Comments
@doowb you've already seen some of this, thanks for the feedback and help with fleshing out some details! |
oo yeah, again you pushed something before me ;d that semester session, nah. I had plans to rewrite the whole stack around and because that i fork some libs in vinyljs org and organize the process in my mind. but yea, i'll review it later! thanks for mention. |
@jonschlinkert mm okey, looks great and thanks for the push up.
👍 that's my thoughts
Or var glob = require('glob-fs')({gitignore: true})
glob.readdir('*.js', {foo: 'bar'}, function (err, files) {
console.log(err || files)
})
.then(console.log)
.catch(console.error) it would work both in same time or separately when only callback is given or when only then/catch. And this will drop I can try to do new iteratorStream with my custom readable stream approach. which can be as standalone module. |
@jonschlinkert are you thinking that module to pass some/all Im passing 95% of the |
I thought I was on async, but @doowb and I were thinking it might not be necessary with stream. but maybe we should just do it to be consistent across the iterators... the fact that you brought it up after we wondered the same thing probably answers the question.
yeah we can optimize after we get the conventions down. I'm much more focused on making sure that we offer something innovative and uniquely valuable with this approach first
still linting... there are actually a few files that aren't even used. some of this is mentioned on the TODO list
so, pattern matching in general has never been the hard part. it's deciding when to include or exclude that's hard. or rather, hard to make a convention around. I can elaborate when get a little further.
also mentioned on readme :) (no worries, there's a lot there, and it's not finished yet. I don't expect you to notice it all)
that's easy, there aren't any here yet lol. I have a bunch but haven't copies them over to this project yet. I re-created the lib like 4 times and just didn't get to that yet
See this section. Middlewares should always return the
no worries, I think I have a good way to do this... for now we should just make this work for a single pattern so the focus stays on middleware conventions. then we can fold in the multi-pattern support.
I'm somewhat of a purist, so I'd probably prefer just adding another iterator. @doowb and I talked about just allowing the user to register iterators like we do with loader-cache. that approach has been working well for us... either way it would be great to have you add an iterator. as your own lib is great, or as a pr. it's totally up to you. that said, if there are ideas for improving existing iterators, I'd love to see that as prs so we can optimize those.
no. I agree this is not a good goal to have. I created this module explicitly to not have to support every edge case under the sun in the core module. middleware can handle edge cases, so those unit tests would be up to the implementors. we just need to figure out how much is enough and go from there... as long as the iterators/handlers and inclusion/exclusion logic actually works, its trivial for a user to support any little feature by just defining an inline middleware, like: var glob = require('glob-fs')({ gitignore: true })
.use(function (file) {
// do stuff
return file;
}); |
awesome. yea maybe it was stupid question, when we have "middleware" idea here. lol, damn im tired.
yea, i read the readme 2 times, lol. And that's clear. The problem is that when pattern is regex
yea, definitely it would be great |
no, not at all. any question is okay. about the matching, yeah we need to optimize that |
haha. Also it would be awesome if we use |
Awesome! Lol yeah I love that lib too. |
Another thinking, when i'm looking latest updates to Hm.. or maybe better would be to be just api normalizer (i mean like consolidate, jstransformers and etc) and we will just implement more and more middlewares and iterators/loaders. So this module will just handle filepaths loading ( I think the idea here in that "first pass" is great and the way is great, but also think we should concentrate in absolute core idea - matching, multiple support, performance, exclusion and inclusion and etc. Then we will just give it to |
hah. exact what i was thinking what about that? // glob-fs (second shot ;d)
var glob = require('glob-fs')()
var opts = {
dot: true,
matchBase: true
ignore: 'baz/foo.txt'
}
glob
.enable('multiple')
.enable('recurisve')
// .option('type', 'stream')
.option('type', 'async')
.option('ignore', 'baz/qux1.txt')
.option('ignore', 'baz/qux2.txt')
.option('ignore', ['baz/s*.txt', 'foo/bar.js'])
// will load some default iterators/loaders
// which can be most used, and we just can focus on core
// .use(globby)
.on('error', console.log)
.on('file', console.log)
.on('dir', console.log)
// it always will be just `.readdir` method
.readdir(['foo/*.js', 'baz/*.txt'], opts, function (err, files) {
// `err` should be always `null` here?
// cuz it would be handled by central mechanism?
}) in given fs tree
|
@doowb / @tunnckoCore
I just pushed up a first pass at this. there is still a lot of work to be done but I think the basic conventions are a good start. I'd love feedback on the following:
file
object in glob-fs, since it really simplifies options handling, etc. I'm thinking that we might be able to implement some interestingignore
/unignore
features that leverage this. maybe even something like thestash
in jade/css/stylus.any other thoughts would be great!
The text was updated successfully, but these errors were encountered: