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

Suggestion: Allow arguments to be differently typed #817

Closed
darnir opened this issue Jan 17, 2017 · 5 comments
Closed

Suggestion: Allow arguments to be differently typed #817

darnir opened this issue Jan 17, 2017 · 5 comments

Comments

@darnir
Copy link

darnir commented Jan 17, 2017

Currently all arguments are always stringly typed. However, in some cases, it makes sense for the argument parser to return data of a different type. For example, my use case is this:

The user passes an XML file name as a parameter. While parsing the arguments, I would like to validate if the file exists, is readable and has the expected schema. I would also like to parse the file at this point, extract the relevant data and store it in a struct. Hence, when the application calls matches.get_value(arg), it returns the struct for the application to use. This allows keeping some of the logic separate and not polluting the actual program with such sanity checking logic.

Unfortunately I do not have a suggestion on the API or how it can be designed. I am only starting to learn Rust and came across this is a feature that I would like in my application code

@BurntSushi
Copy link
Contributor

This allows keeping some of the logic separate and not polluting the actual program with such sanity checking logic.

Well, you have to write that logic somewhere. :-) If you want to keep it separate, you can use any of Rust's abstraction facilities to tuck that logic away. For example:

struct ArgMatches<'a>(clap::ArgMatches<'a>);

impl<'a> ArgMatches<'a> {
    fn get_xml_struct(&self) -> Result<XMLStruct, Error> {
        let file_path = self.0.value_of("xml-file").unwrap();
        ...
    }
}

@kbknapp
Copy link
Member

kbknapp commented Jan 18, 2017

This has been on my mind since back in #146. There are some macros for doing this, but it's still a less than ideal solution. Aside, this is something that @BurntSushi's library docopt does really well and he's being super modest by not saying anything 😉

The biggest issue with doing this is the intermediate step of ArgMatches where you have to store something, which is currently OsStrings. In order to get a typed value out it'll have to be something that implements a FromStr or similar trait and would either give back a trait object or do the conversion (which is what the value_t! macros do).

What I'd like to implement, and have been passivly waiting on macros 1.1, is a #[derive(Args)] (or similar) which converts the ArgMatches to some typed Args struct.

struct Args {
    file_path: Option<XMLStruct>
}

The one downside is you'd still have Option<T>s becasue even with required arguments, there's still scenarios where they may not be used (due to certain settings, overrides, etc.). But it's still better than what we have today. Of course, it'd be possible to do today with some other macro (vice a derive derective), but I'd really like to keep the consumer API simple-ish and only require them to add derive attribute, nothing else.

@BurntSushi's way is great too, and some also do a From<ArgMatches> impl

struct Args {
    file_path: XMLStruct
}

impl<'a> From<ArgMatches<'a>> for Args {
    fn from(m: ArgMatches) -> Self {
        let file_path = m.value_of("xml-file").unwrap();
        // ...
        Args { files_path: file_path }
    }
}

Which is essentially the same thing, but allows you to do

let args: Args = App::new("") 
    // stuff
    .get_matches()
    .into();

@quodlibetor
Copy link
Contributor

The one downside is you'd still have Options becasue even with required arguments, there's still scenarios where they may not be used (due to certain settings, overrides, etc.).

What is a situation where a combination of overrides and settings overrules requires? This seems like someone doing a poor job defining requires, but I admit I don't have the best imagination.

It would be really nice if derive(Args) resulted in a init() -> Result<ArgStruct, clap::Error> method that meant that it's possible to have really-genuinely-required parameters in at least some positions.

This might require a combination of defaults, "requires", and matching subcommands to (optional) embedded structs instead of flattening everything into a single namespace, but it feels in principle doable.

@kbknapp
Copy link
Member

kbknapp commented Jan 30, 2017

@quodlibetor I apologize I missed your comment.

What is a situation where a combination of overrides and settings overrules requires? This seems like someone doing a poor job defining requires, but I admit I don't have the best imagination.

When requirements are overriden by subcommands or other arguments, and conditional requirements. This probably doesn't come up often and could probably be avoided by some CLI redesign, but still it could happen. The primary way I can think of to avoid this is make it clear that "required" args if overriden, etc. must implement Default and therefore have at least a default value (Rust default, not clap default).

Plus there are things like some CLIs where the subcommand is required, and others it's not. There are also CLIs where the use of a subcommand negates any args of the partent, to include required args.

It would be really nice if derive(Args) resulted in a init() -> Result<ArgStruct, clap::Error> method that meant that it's possible to have really-genuinely-required parameters in at least some positions.

I also think there should be two different conversion, one that can fail (returning a Result) and one that can't. Similar to From and TryFrom.

@kbknapp
Copy link
Member

kbknapp commented Jan 30, 2017

I'm going to close this issue as it's covered by two other open issues

@kbknapp kbknapp closed this as completed Jan 30, 2017
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

4 participants