Skip to content
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

Deprecate glob in favour of globset and globwalk? #17

Open
KodrAus opened this issue Apr 9, 2018 · 8 comments
Open

Deprecate glob in favour of globset and globwalk? #17

KodrAus opened this issue Apr 9, 2018 · 8 comments
Labels
crate Related to a library as opposed to a resource tracking issue

Comments

@KodrAus
Copy link
Collaborator

KodrAus commented Apr 9, 2018

The glob crate has some deficiencies that are addressed by globset, but it comes with its own set of trade-offs. The globwalk crate offers a glob function, based on globset, that's a missing piece of the globset API.

There's a willingness to deprecate glob in favour of globset and accept its trade-offs as facts of life that can be improved by the compiler.

We probably need someone to take up the cause and help push this forward.

@KodrAus KodrAus added the crate Related to a library as opposed to a resource label Apr 9, 2018
@BurntSushi
Copy link
Member

BurntSushi commented Apr 17, 2018

A possible path forward here (although, my personal bandwidth for this is super limited):

  • Don't deprecate glob, but re-purpose it. The name is ideal. Instead, we might consider deprecating globset and moving its API wholesale into glob. Then we produce a new breaking glob release. This path is useful if we care about moving the ecosystem over. It's less useful if we don't care about that as much.
  • My suspicion is that folks expect "globbing" and "glob the file system" to live very close together. It would be nice if they were in the same crate. I had some thoughts here previously. In particular, whatever the case, it should be possible to get "globbing" without the dependencies required for walking the file system.

I think a key issue here is that "glob the file system" needs to be proved out and polished.

At a higher level, it's also worth saying that globset (or glob, if we took that route) will live on and get maintenance support so long as ripgrep is alive, so I do think it makes sense to take advantage of that!

@KodrAus
Copy link
Collaborator Author

KodrAus commented Apr 29, 2018

Thanks for your thoughts @BurntSushi!

Personally, I'm in favour of re-purposing the glob crate as globset's current API, and shifting libraries into places where they get greater than zero on-board maintainers is what this is all about! :)

I'm guessing we're not in a big hurry to do this, so could work towards proving and polishing the glob the filesystem case in globwalk before repurposing glob? The main point from @BurntSushi's list for me is the first one about nailing down a specification for how traversal should behave that gives us room to optimise the implementation. Maybe that's the next step here?

@Gilnaa Do you have any thoughts to share? I know it hasn't actually been very long since you started working on globwalk.

@Gilnaa
Copy link

Gilnaa commented Apr 30, 2018

I am not against breaking changes, per se, but removing file-system traversal from glob is a funny move, as it doesn't just change the API, but the purpose of the crate.

It might be a viable move once we prove globwalk's API is sufficient, but I am still a bit weary of it.

@KodrAus
Copy link
Collaborator Author

KodrAus commented May 13, 2018

@Gilnaa I agree it would be a bit strange to do that. I'm assuming we'd effectively deprecate both globset and globwalk, by combining them in glob with a feature gate for directory traversal.

It would be good if we could get concrete about what proving globwalk's API sufficient means.

@Gilnaa
Copy link

Gilnaa commented May 14, 2018

Well, globset's API is a superset of glob's, so it's probably not that big of a deal; but if we could get some community feedback about it would be great.

@kinggoesgaming
Copy link

I am in favour of glob being repurposed and the other two crates being deprecated

@Dylan-DPC-zz
Copy link
Collaborator

Yeah I am in favour of having a single crate for both. We can use feature gates in case people would want only 1 of the 2 functionalities.

@BatmanAoD
Copy link

If glob were repurposed, and we maintained separate crates for just globbing (i.e. globset) and globbing the filesystem (globwalk), my vote would be to repurpose glob as a globwalk alternative (either based on globset + traversal, or simply based on the existing globwalk), and leave globset with its current name.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate Related to a library as opposed to a resource tracking issue
Projects
None yet
Development

No branches or pull requests

6 participants