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

Add directory arg support #23

Closed
jayvdb opened this issue Jul 17, 2023 · 7 comments
Closed

Add directory arg support #23

jayvdb opened this issue Jul 17, 2023 · 7 comments

Comments

@jayvdb
Copy link
Contributor

jayvdb commented Jul 17, 2023

Would it make sense to expand this crate to include directory args?

I created a simple --path arg at Electron100/butane#130 , and it was not simple to do with builder pattern, and needs to be more complicated to be correct. It should be easier, and a clio helper could provide it.

Then there are variations for whether the directory should exist already, or the user has sufficient permissions to create it.

@aj-bagwell
Copy link
Owner

aj-bagwell commented Jul 17, 2023

The orignial motivation of this crate was to handle files, but I think directory support makes.

It has some basic support in ClioPath and the clap validaotor already, e.g. you can get clap to validate that a path is a directory that exists like so:

use clap::Parser;
use clio::ClioPath;

#[derive(Parser)]
#[clap(name = "foo")]
struct Opt {
    /// path to directory
    #[clap(value_parser = clap::value_parser!(ClioPath).exists().is_dir(), default_value = ".")]
    log_dir: ClioPath,
}

fn main() {
    let opt = Opt::parse();

    println!("log dir is: {}", opt.log_dir);
}
$: foo
log dir is: "."
$: foo /tmp/
log dir is: "/tmp/"
$: foo /tmp/not-exists
error: Invalid value for <LOG_DIR>: Invalid path "/tmp/not-exists": No such file or directory (os error 2)
$: foo /tmp/file
error: Invalid value for <LOG_DIR>: Invalid path "/tmp/file": Not a directory (os error 20)

What were you imaginging? A specific type so you could just use #[clap(value_parser)]?

@jayvdb
Copy link
Contributor Author

jayvdb commented Jul 18, 2023

Nice. Any chance you can provide a sample for how to do that with the "builder" clap pattern?

I have everything working except this bit:

            Arg::new("path").short('p').long("path")
            .default_value(WORKING_DIR_PATH.as_os_str())
            .value_parser(value_parser!(ClioPath))

error

error[E0599]: the method `value_parser` exists for reference `&&&&&&_AutoValueParser<ClioPath>`, but its trait bounds were not satisfied
    --> butane_cli/src/main.rs:23:27
     |
23   |             .value_parser(value_parser!(ClioPath))
     |                           ^^^^^^^^^^^^^^^^^^^^^^^ method cannot be called on `&&&&&&_AutoValueParser<ClioPath>` due to unsatisfied trait bounds
     |
    ::: /home/jayvdb/.cargo/registry/src/index.crates.io-6f17d22bba15001f/clap-4.1.6/src/builder/value_parser.rs:2187:1
     |
2187 | pub struct _AutoValueParser<T>(std::marker::PhantomData<T>);
     | ------------------------------ doesn't satisfy `_: _ValueParserViaParse`

It would be useful to have helpers for a "base directory" arg. i.e. encapsulate the lazy_static needed to get std::env::current_dir() into a clap default_value.

@aj-bagwell
Copy link
Owner

aj-bagwell commented Jul 18, 2023

You have the right code, you just need to enable the clap-parse feature for clio in your Cargo.toml. You can add the validation option to the parser returned by value_parser!(ClioPath).

Also if you enable the string fearute on clap, then you can use an OsString as a default value so you don't need a static ref, so no need for lazy static.

That said I am not sure that I see the value in current_dir().unwrap_or(".".into()) as the main value of current_dir() is in the error if the current_dir does not exists/has wrong permissions, as for the default shown in the help I think ./ is more expressive that showing the whole path of the current directory.

I am probably thinking about this too much but it did make me raise #24 as with . it can both return true for exists() but still return not found when you try to open it. It also made realise that the error reporting when the default is invalid is terrible, so I may raise a clap issue about that.

Here is a full example incase it helps.

[package]
name = "hello"
version = "0.1.0"
edition = "2021"

[dependencies]
clap = { version = "4.1", features = ["string"] }
clio = { version = "0.3.2", features = ["clap-parse"] }
use clap::{Parser, value_parser, Arg};
use clio::ClioPath;
use std::path::PathBuf;

pub fn working_dir_path() -> PathBuf {
    match std::env::current_dir() {
        Ok(path) => path,
        Err(_) => PathBuf::from("."),
    }
}

fn main() {

let app = clap::Command::new("hello")
        .version(env!("CARGO_PKG_VERSION"))
        .arg(
            Arg::new("path").short('p').long("path")
            .default_value(working_dir_path().into_os_string())
            .value_parser(value_parser!(ClioPath).exists().is_dir())
        );
	let args = app.get_matches();

	let log_dir = args.get_one::<ClioPath>("path").unwrap().clone();

    println!("log dir is: {}", log_dir);
}

@aj-bagwell
Copy link
Owner

Turns out there is already clap-rs/clap#4643 "assert_defaults assumes parsers are stateless"

@aj-bagwell
Copy link
Owner

The other issue that may cause that error is that cargo may be including both clap 3 and clap 4 and clio then implements the clap 3 _AutoValueParser but clap is expecting it to implement the clap 4 _AutoValueParser, in that case I resort to updtaing the Cargo.lock file manullaly.

@jayvdb
Copy link
Contributor Author

jayvdb commented Jul 19, 2023

Adding feature clap-parse did the trick.
Sooo.. I suggest updating the readme to indicate that it does this directory stuff.
There are lots of little gotchas like #24 which make this problem more complex that it seems, which is why a helper is warranted.

@aj-bagwell
Copy link
Owner

Fixed #24 and added a directory example to the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants