Gilnaa / globwalk

MIT License
47 stars 14 forks source link

Increase richness of glob syntax? #3

Closed epage closed 6 years ago

epage commented 6 years ago

I'm working on a file system stager to help make lay out packages. Currently I'm using globwalk for a way to grab files by pattern.

In stager, I want to add exclusion support. I can add exclusions as an additional field but I got thinking, why not use gitignore syntax?

Example: ["*.rs", "!/tests/**/*.rs"] would find all rust source except in a test directory.

epage commented 6 years ago

The idea is to take all paths returned by WalkDir, make them relative to the WalkDir root, and if there is a match from ignore::GitIgnore, then return it to the user

If this isn't something globwalk is interested in expanding into, I'll probably create a parallel crate globtrotter that does. In some respects, I'm hoping you want to keep globwalk simple because I like the name globtrotter ;).

Gilnaa commented 6 years ago

I'm hesitant to implement this as you suggest since I've been careful not to inspect the strings passed to globset.

This might be achieved by using the filteriterator adapter

epage commented 6 years ago

My thought isn't to use this (EDIT: ignore) on top of globset but to use ignore instead of globset.

Gilnaa commented 6 years ago

I'm not sure it is possible to manually add glob-patterns to ignore without using an ignore file.

It is possible to use ignore, but it would still require usingglobset, and unless I've misunderstood something, it won't give you the features you want

epage commented 6 years ago

I'm not sure it is possible to manually add glob-patterns to ignore without using an ignore file.

While the top level API of ignore deals with files on disk, the gitignore module works entirely in-memory. I'm already using it in cobalt for advanced blacklisting

Gilnaa commented 6 years ago

Oh, I see, I'll look into it

Gilnaa commented 6 years ago

I can't seem to make gitignore work with the existing API, and tbh I don't have much time to work on it right now. You're welcome to open a PR but I don't think I'll be able to work on it.

Good day, :)

epage commented 6 years ago

Oh, I was assuming I would until it sounded like you were going to jump on it. It might be a few days.

Gilnaa commented 6 years ago

No worries, take your time :)

On Thu, Apr 19, 2018, 20:03 Ed Page notifications@github.com wrote:

Oh, I was assuming I would until it sounded like you were going to jump on it. It might be a few days.

— You are receiving this because you commented.

Reply to this email directly, view it on GitHub https://github.com/Gilnaa/globwalk/issues/3#issuecomment-382809440, or mute the thread https://github.com/notifications/unsubscribe-auth/ABRx0wkCAdhLG8Qe8uihUihY1w0QK47dks5tqMNNgaJpZM4TTxEP .

epage commented 6 years ago

I see two general options for using ignore

How do you feel about changing the API? Any preference on route?

Gilnaa commented 6 years ago

I'd rather take the second option.

Maybe it's possible to replace the IntoIter impl with a build(self)->Result<Iter> or something.

@killercup I'd love to hear your 0.016€ (as of 21:10 19/04/2018 UTC)

epage commented 6 years ago

So in looking at the different APIs and considering the trade offs, here are some random thoughts:

I personally lean towards globwalks original API / Gitignores API (::new(base)) rather than globwalks current API (base is defaulted).

I didn't even realize globwalks API changed until I started implementing this. I got confused; I didn't remember the API looking like this :)

Gilnaa commented 6 years ago

Yes, the GlobWalk API was changed to mirror that of glob's.

I agree in that the base case of "iterate from CWD" is unlikely and probably a bad practice. Personally I don't see myself using it when writing a CLI application.

Saying that, I'd rather we keep enough of the current API to be mostly a drop in replacement for glob. I wouldn't mind for a change as long as the freestanding glob function remains the same.

epage commented 6 years ago

I'd rather we keep enough of the current API to be mostly a drop in replacement for glob

Except a drop-in replacement for globs relative or absolute behavior? Or do you plan to find a way to attempt to do both?

Gilnaa commented 6 years ago

I've just realised I'm making no sense. It's possible the have glob implemented by means of the API you propose, so I have no objectiond really  ¯_(ツ)_/¯

killercup commented 6 years ago

@Gilnaa, I think I can even spare 0.019 CHF, I still have some from RustFest.

I'm a huge fan of the pattern syntax used in .gitignore files, which is also becoming more popular in other areas like file search in editors.

I'm not sure I get your discussion on glob-crate-like-API instead of a base path argument? Having a new as well as a new_from_cwd sounds like no big deal to me. Having both also makes it obvious what the default is.

epage commented 6 years ago

In preparing my change, do you mind if its re-formatted with rustfmt 1.25 (dedicated commit, of course)? Or should I disable my editors auto-formatting?