Deno: FS glob pattern

[zekth]

In NodeJs I used to often import glob modules to use glob patterns for parsing filesystem. Is it in the philosophy of deno to have it in the fs module?

Is it worth it that i work on an implementation of it?

[kevinkassimo]

Pretty sure it will be very useful, and I don't think there is an implementation yet for Deno. Would be a nice addition to https://github.com/denoland/deno_std/tree/master/fs if you want to work on this

[zekth]

The WalkOptions.match uses regexp so i think we can extend it in some way. Like detect if the provided pattern is a glob or a regex

[hayd]

See also https://github.com/hayd/deno-globrex

[zekth]

@hayd nice implementation there is some work to do to implement in the walk funcs of deno but could be nice.

Just a question, in term of implementation. Is it better to parse glob pattern to have a regex and keep the code like this?

PS: i laughted so hard at your unit tests

[hayd]

The unit tests are copied from the original :) hence the weirdness.

Edit: ha, I'd not noticed the tests were NSFW!

Perhaps the API should be cleaned up for std, IMO it's a little goofy. Perhaps a function glob that takes string and options and returns regex?

It would also be nice to have inclusion of this for https://github.com/denoland/deno_std/issues/193 (though using a regex for test|.*_test is okay initially) and for --fmt.

[zekth]

Agree. I think something like this could be nice:

if (options.glob && ! options.match) {
    options.match = glob2regex(options.glob);
  }

In this case the only parsing is on the regex. So no change on the actual code base. But problem is on each walk we add another condition; which is not really good.

[hayd]

For fs walk it already takes a regex, so I was imagining first glob to regex, then pass that regex to walk. Hence I think std should export:

glob(glob: string, options?:GlobOptions): RegExp

[zekth]

Fair point. In this case we can describe GlobOptions to be used and revelant for the walker like:

export interface GlobOptions{
  onError?: (err: any) => void;
  followSymlinks?: Boolean;
}

This way we can do something like:

function glob(glob: string, options?:GlobOptions):AsyncIterableIterator<FileInfo>{
  let regex = glob2regex(glob);
  return walk(".", {match: regex});
}

Not sure about the cwd btw. We might need to detect it in the glob2regex.

[hayd]

I don't think we should merge glob (get RegExp) + walk (get Iterator). Although I agree passing the RegExp to walk as match is often what you'll be doing, it's better to be explicit.

So GlobOptions instead of
{extended = false, globstar = false, strict = false, filepath = false, flags = ''}

I wonder if cwd is related to the filepath arg...?

[zekth]

As in the doc we can see:

 * @param {Boolean} [opts.filepath=''] Parse as filepath for extra path related features

So i think it's not related to cwd. But if we take your point of view we don't face the problem of the cwd because the call would be:

walk('./mydir',{regex:[globrex('**/*.ts')]})

Is that correct for you?

[hayd]

walk('./mydir', { match: glob('**/*.ts') }) (no need for walk api change)

So perhaps a way forward is:

copy over to fs:
globrex.ts
globrex_test.ts
add a friendly glob wrapper as discussed above:
glob.ts
glob_test.ts

?

[zekth]

Totally agree with this. Working on it.

[hayd]

(maybe remove the swear words from the tests, cough!)

[zekth]

I've pushed on my fork but it appears we have 2 problems in the globrex:
https://github.com/zekth/deno_std/tree/glob

1. https://github.com/zekth/deno_std/blob/glob/fs/glob_test.ts#L89 => regex generated: /^x..*$/
   in this case it does not work. But the regex match: https://regex101.com/r/nTtDvB/1
2. https://github.com/zekth/deno_std/blob/glob/fs/glob_test.ts#L103 = regex generated: /^\/a\/.\/x..$/
   Same kind of case: https://regex101.com/r/hcfwch/1

If think there is maybe an issue with the regex matches. You can see here: https://github.com/zekth/deno_std/blob/glob/fs/glob_test.ts#L29

The patterns are working.

Also for example the glob('a/*/.ts') generates: ^\/a\/.*\/x\..*$ which does not match with the path ./a/yo/x.ts for example.

So you think we need to explicitely specify ./ or we do a fallback for this?

[hayd]

Do you need to pass globstar = true?

[zekth]

So globstar definitely helps and also the flag:'g' too to avoid the problems with the root './'

But i'm experiencing a strange problem now. on : match: glob(join( "a", "**", "*.ts"), { flags:'g', extended:true, globstar: true })

the output from walk and walksync are not the same. Walksync output is empty.
On this test: https://github.com/zekth/deno_std/blob/glob/fs/glob_test.ts#L106

Maybe you have an idea? I've tried many things and using the flag and the globstar is the cleaner way to do it. Maybe i've found an issue in the walkersync?

[kitsonk]

Isn't this a better conversation to have on deno_std?

[hayd]

yep, moved there. please close here @zekth

[terkelg]

@zekth @kitsonk I'm the author of globrex. Happy to help. I'm aware of the issues and are currently working on a new and improved version written in TypeScript. Let me know if I can help.

[nayeemrmn]

@terkelg Thanks! We now have a globToRegExp() which wraps globrex (and an optimised expandGlob() which I see was discussed here).

I want to point out a patch I made to globrex as used here: https://github.com/denoland/deno_std/pull/604/files#diff-3. I know it fixes Windows separator issues and possibly a bit more, unfortunately I don't remember the precise justifications... I wish I had added relevant tests 😔. Hope it helps anyway if you can review it.

[terkelg]

Thank you @nayeemrmn – sounds great. I'm in the middle of reworking tiny-glob which uses Globrex. It turns out that there are a lot of edge cases globrex doesn't take into account. I therefore decided to split globrex into two modules: A robust glob to AST parser and a AST to RegExp generator.
The goal is to still keep it simple not use any external libs, but It's slightly more complex than globrex.

I don't know what the best approach is and if there are any constraints for what can make it into the Deno std? However, when done I can do a custom rewrite that combines the glob parser and RegExp generator for Deno if needed be? I'm pretty keen on contributing and making sure we get this right.

[zekth]

As deno_std is dependency free it will still be recopied from your repo to deno_std one. I think @hayd can speak for how he ported it. AFAIK it was mainly the test wrapper which was different to handle and add interfaces. Happy to read your will to contribute.