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

Tracking issue CustomDerive #1661

Closed
9 of 27 tasks
kbknapp opened this issue Nov 12, 2017 · 16 comments
Closed
9 of 27 tasks

Tracking issue CustomDerive #1661

kbknapp opened this issue Nov 12, 2017 · 16 comments
Labels
A-derive Area: #[derive]` macro API
Milestone

Comments

@kbknapp
Copy link
Member

kbknapp commented Nov 12, 2017

From @kbknapp on July 6, 2015 1:22

Comments have been cleaned up and updated. This thread should be related to general progress towards 1.0 for this crate.

To Do

  • #[derive(FromArgMatches)]
    • Implemented
    • Docs
    • Tests
    • Examples
  • #[derive(IntoApp)]
    • Implemented
    • Docs
    • Tests
    • Examples
  • #[derive(ClapApp)] (Automatically does both FromArgMatches and IntoApp)
    • Implemented
    • Docs
    • Tests
    • Examples
  • #[derive(ClapSubCommands)]
    • Implemented
    • Docs
    • Tests
    • Examples
  • #[derive(ArgEnum)]

Reference

Derive FromArgMatches

#[derive(FromArgMatches)]
struct MyApp {
    verb: bool
}

fn main() {
    let m: MyApp = App::new("test")
        .arg_from_usage("-v, --verbose 'turns on verbose printing'")
        .get_matches()
        .into();

    println!("Verbose on: {:?}", m.verb);
}

Derive IntoApp

/// Does stuff
#[derive(IntoApp)]
struct MyApp {
    /// Turns on verbose printing
    #[clap(short = 'v', long = "verbose")]
    verb: bool
}

Derive Subcommands

#[derive(ClapSubCommands)]
pub enum Commands {
    Test(Test),
}

Derive TryFromArgMatches

#[derive(TryFromArgMatches)]
struct MyApp {
    verb: bool
}

fn main() {
    let m: Result<MyApp, clap::Error> = App::new("test")
        .arg_from_usage("-v, --verbose 'turns on verbose printing'")
        .get_matches()
        .try_into();
}

Derive ArgEnum

#[derive(ArgEnum, Debug)]
enum ArgChoice {
    Foo,
    Bar,
    Baz,
}

fn main() {
    let matches = App::new(env!("CARGO_PKG_NAME"))
            .arg(Arg::with_name("arg")
                .required(true)
                .takes_value(true)
                .possible_values(&ArgChoice::variants())
            ).get_matches();
    
    let t = value_t!(matches.value_of("arg"), ArgChoice)
        .unwrap_or_else(|e| e.exit());

    println!("{:?}", t);
}

Copied from original issue: #146

@kbknapp
Copy link
Member Author

kbknapp commented Nov 12, 2017

@kamalmarhubi

I'd love for this to land within the 1.15 cycle!

Absolutely, me too! If we can resolve those outstanding questions above (values in order, ensuring T is bound with Default if not using Option<T>) and ensure the naming conventions of the APIs are inline with each other I don't see why this couldn't happen!

The subcommand enum not being inlined I'm fine with.

@Nemo157 and @kamalmarhubi

I forgot to mention in the last post the example above merges two distinct ideas into one, but I'd also want the ability to do one or the other and not be forced to use both. I haven't dug into the source yet, so if it's already possible ignore this comment 😜

What I mean is, I view both of these as distinct ideas (also, I'm using clap names in this example, but if it remains in the stomp crate, it'd be stomp):

  • Create an App struct from MyApp using the #[clap(short='c', long="config")]
  • Create MyApp struct from ArgMatches using the #[derive(ClapCommand)]

Being able to do these two things separatly would greatly increase adoptability. i.e. "I've already got an App, so now I just write out my MyApp struct and place a #[derive] attribute on there and I'm off to the races.

Likewise, if for some unknown reason they don't want to use the ArgMatches->MyApp conversion, but still want to take advantage of the MyApp->App they could. Although I see this as less beneficial merely a biproduct.

@kbknapp
Copy link
Member Author

kbknapp commented Nov 12, 2017

From @Nemo157 on January 30, 2017 7:24

😃 I would definitely be happy to make this a part of clap proper.

look through the entirity of the code to ensure it matches all edge cases and such

At the moment, definitely not. I was basically using my application and the primary clap example as a feature list for what to implement, so there are many things missing.

Using T instead of Option for requirements could be dangerous when things like requirements get overriden, conditional requirements, etc.

Yeah, again because of the test cases I was using I did not consider that at all. Hopefully, at least for the case where the user both generates the App and the conversion from ArgMatches, it should be possible to detect issues at compile time.

The subcommands portion would be even nicer if the enum could be specified inline

Depends which way you mean, an anonymous enum in the parent command would be cool, but probably not really doable without proper anonymous enum support in Rust. An enum with struct variants definitely should be possible, it would result in some massive generated code blocks, but that shouldn't be too bad, and could be fixed in the future with types for enum variants allowing delegating variant specific parsing.

I forgot to mention in the last post the example above merges two distinct ideas into one, but I'd also want the ability to do one or the other and not be forced to use both.

The trait doesn't allow it, but the macro is basically following two distinct paths for each so would be simple to split the trait for it.

Also, one thought I had soon after I implemented this was that the macro is doing too much. It should be possible to split a lot of what the macro is doing based on types (which is actually based on type names, so could very easily be broken with type aliases etc.) out to trait implementations. For example something like trait FillArg { fn fill_arg(arg: Arg) -> Arg } (superbad name 😞) for filling out the details derived from the type and something else used during parsing.

I think I should have some time this week I could spend on this, I can definitely try and do a quick port into the clap codebase and do a bit of documentation on how it currently works and what's missing.

@kbknapp
Copy link
Member Author

kbknapp commented Nov 12, 2017

So the example would go to:

/// Does awesome things.
#[derive(ClapApp)]    // Does both IntoApp, and FromArgMatches
#[clap(name = "MyApp", version = "1.0")]
#[clap(author = "Nemo157 <clap@nemo157.com>")]
pub struct MyApp {
    /// Sets a custom config file
    #[clap(short = "c", value_name = "FILE")]
    config: Option<String>,

    /// Sets an optional output file
    #[clap(index = "1")]
    output: Option<String>,

    /// Turn debugging information on
    #[clap(counted, short = "d", long = "debug")]
    debug_level: u64,

    #[clap(subcommand)]
    subcommand: Option<Commands>,
}

#[derive(SubCommands)]
pub enum Commands {
    Test(Test),
}

/// does testing things
#[derive(ClapApp)]
pub struct Test {
    /// lists test values
    #[clap(short = "l")]
    list: bool,
}

@omarabid
Copy link

@kbknapp Have you stopped work on this? The derive/struct approach is a game changer. I can see a substantial productivity boost from having to define a struct and clap deriving the required parsing for it.

@Dylan-DPC-zz
Copy link

@omarabid yes Kevin has been busy. It doesn't mean he has stopped working on it, but there are other priority issues to tackle. If you are free, you can send us a PR implementing any one of the above features and we will be happy to review it. Thanks

@omarabid
Copy link

@Dylan-DPC I can't promise as I just started learning Rust. On the other hand, if I were to start contributing to OS projects it'll definitely be this.

@Dylan-DPC-zz
Copy link

@omarabid no issues. we can help you if you are stuck on anything.

@TeXitoi
Copy link
Contributor

TeXitoi commented Feb 24, 2019

@omarabid while waiting for clap_derive to be finished, you can use https://crates.io/crates/structopt

@pksunkara pksunkara transferred this issue from clap-rs/clap_derive Feb 3, 2020
@pksunkara
Copy link
Member

@CreepySkeleton This is also related to custom traits.

@pksunkara pksunkara added A-derive Area: #[derive]` macro API W: 3.x labels Feb 3, 2020
@pksunkara pksunkara added this to the 3.0 milestone Feb 3, 2020
@CreepySkeleton
Copy link
Contributor

CreepySkeleton commented Feb 3, 2020

Yeah, essentially the same design I've been thinking about 👍

@pksunkara pksunkara removed the W: 3.x label Feb 3, 2020
@TeXitoi
Copy link
Contributor

TeXitoi commented Feb 3, 2020

For the ArgEnum thing, you may also be interested by https://crates.io/crates/strum it lacks almost nothing to be just enough.

@Nukesor
Copy link
Contributor

Nukesor commented Oct 30, 2020

Hey :)

I'm just in the middle of trying the new Clap v3 derive setup.
My old cli setup has been built with StructOpt v0.3 and I'm super hyped for Clap v3.

However while migrating, I stumbled upon a little problem. Everything worked perfectly fine until I tried to use the new derive system in combination with the new shell completion generation functions.

My current approach is something like this:

use clap::Clap;

#[derive(Clap, Debug)]
pub struct Opt {
    /// Verbose mode (-v, -vv, -vvv)
    #[clap(short, long, parse(from_occurrences))]
    pub verbose: u8,
}

async fn main() -> Result<()> {
    let opt = Opt::parse(); // No longer works without `Clap`
    // let opt = Opt::into_app(); // Only works with `IntoApp` derive.
    //generate_to::<Bash, _, _>(&mut app, "pueue", "/tmp".into());
}

generate_to requires an &mut App<'_>.
It's possible to get an App from Opt with the IntoApp derive, but one cannot derive from Clap and IntoApp at the same time.

error[E0119]: conflicting implementations of trait `clap::IntoApp` for type `cli::Opt`:
   --> client/cli.rs:311:10
    |
311 | #[derive(Clap, IntoApp, Debug)]
    |          ^^^^  ------- first implementation here
    |          |
    |          conflicting implementation for `cli::Opt`
    |
    = note: this error originates in a derive macro (in Nightly builds, run with -Z macro-backtrace for more info)

So here's my questoin:
What's the best way to get the new struct based declaration and usage, while also being able to generate shell completion files.

There's probably just something I'm overlooking.

Thanks in advance :)

@pksunkara
Copy link
Member

You don't need to specify IntoApp derive. Clap derive automatically does that. Just make sure that IntoApp trait is in use for you to use the into_app function

@TilCreator
Copy link

TilCreator commented Apr 9, 2021

I really like the derive stuff, it's great!
But I found a problem with #[clap(parse(try_from_str)]:

/*
Available functions:
reqwest::Url::try_from(s: &'a str) -> Result<Self, Self::Error>
reqwest::Url::from_str(input: &str) -> Result<Url, crate::ParseError>
scraper::Selector::try_from(s: &'i str) -> Result<Self, Self::Error>
*/

#[derive(Clap, Debug)]
#[clap(name = env!("CARGO_PKG_NAME"))]
struct Args {
    #[clap(
        takes_value = true,
        parse(try_from_str)
    )]
    url: Url,
    #[clap(
        takes_value = true,
        parse(try_from_str),
    )]
    selector: Selector,
}    
error[E0277]: the trait bound `Selector: FromStr` is not satisfied
  --> src/main.rs:30:15
   |
   |         parse(try_from_str),
   |               ^^^^^^^^^^^^ the trait `FromStr` is not implemented for `Selector`
   |
   = note: required by `std::str::FromStr::from_str`

Parsing Args::url works great, but Args::selector doesn't find the from function, because clap doesn't seam to search for try_from implementations. Guessing by the name I would expect try_from_str to use try_from(&str), not from(&str).

@pksunkara
Copy link
Member

Can you please create a separate issue?

@epage
Copy link
Member

epage commented Oct 12, 2021

We've got examples and tests but are still lacking in docs. I've split that out into #2856

If there is anything in this that I missed, let's split it out so we can have concrete steps to move forward.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-derive Area: #[derive]` macro API
Projects
None yet
Development

No branches or pull requests

9 participants