aj-bagwell / clio

A rust library for parsing command line file name arguements
12 stars 7 forks source link

FYI — directories / `clio_extended` #10

Closed max-sixty closed 1 year ago

max-sixty commented 1 year ago

Just an FYI that over at PRQL we wanted our CLI to also be able take a directory, and so added some logic to clio in our crate.

We're very open to feedback on whether we're doing reasonable things!

Here's the code: https://github.com/prql/prql/blob/c1150818f4497c89164ba9ec9468459acb8a191c/prql-compiler/prqlc/src/cli.rs#L409

Feel free to close as soon as you've seen this. Thanks.

aj-bagwell commented 1 year ago

This is a neat idea. I have released version 0.3 which adds a struct for tracking a path (which may be stdin/stdout) and was the perfect place to add a files method to list files in a directory.

So you should now be able to do something like:

    pub fn read_to_tree(path: ClioPath) -> anyhow::Result<SourceTree<String>> {
        let mut sources = HashMap::new();
        for file in path.files(has_extension("prql"))? {
            let mut file_contents = String::new();
            let path = file.path().to_owned();
            file.open()?.read_to_string(&mut file_contents)?;

            sources.insert(path, file_contents);
        }

        Ok(SourceTree::new(sources))
    }

(well ignoring the relative path name tidying up you do)

Does that work? I contemplated making it return a Vec<Input> but worried about it running out of file handles.

max-sixty commented 1 year ago

ClioPath looks great! I think that will work well. Thanks!

I'm CCing @aljazerzen who wrote most of the code on our end if he has any thoughts.

files looks fine — I'm not even sure it's needed; can folks just pass a ClioPath to WalkDir? It would almost be passable exactly (i.e. without a &*path) if it implemented AsRef<Path>, though not sure whether there are other reasons not to do that...

aljazerzen commented 1 year ago

Yup, ClioPath does it: https://github.com/PRQL/prql/pull/2847

max-sixty commented 1 year ago

Wonderful! I'll close...

aj-bagwell commented 1 year ago

Glad it was useful, of you have any other changes you want to make pull request are always welcome.

I didn't implement AsRef<Path> as passing a ClioPath to File::open would then work but defy the entire point of clio which is the - handling.

Passing it to walkdir would have similar issues, whereas the files method returns a Vec containing the original ClioPath if it is not a directory.

To be honest I should probably remove the Deref impl and pick some proper semantics for all Path methods and reimplemented properly.

Is stdin a file? Does it have metadata? Does it exist? What about URLs?

max-sixty commented 1 year ago

Ah, that makes a lot of sense. Thanks for the explanation...