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

[3.1] Derive parsing FromStr args into a vector regression from 3.0 #3514

Closed
2 tasks done
ufoscout opened this issue Feb 26, 2022 · 2 comments · Fixed by #3521
Closed
2 tasks done

[3.1] Derive parsing FromStr args into a vector regression from 3.0 #3514

ufoscout opened this issue Feb 26, 2022 · 2 comments · Fixed by #3521
Labels
A-builder Area: Builder API C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much

Comments

@ufoscout
Copy link

Please complete the following tasks

Rust Version

rustc 1.59.0 (9d1b2106e 2022-02-23)

Clap Version

3.1.2

Minimal reproducible code

use clap::Parser;
use std::str::FromStr;

#[derive(Debug, PartialEq)]
pub enum MyType {
    A,
    B,
}

impl FromStr for MyType {
    type Err = String;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s.to_lowercase().as_str() {
            "a" => Ok(MyType::A),
            "b" => Ok(MyType::B),
            _ => Err("".to_owned()),
        }
    }
}

#[derive(Parser)]
pub struct Config {
    #[clap(long, default_value = "a,b,a", value_delimiter = ',')]
    pub values: Vec<MyType>,
}

// These two tests pass in 3.0 but fail in 3.1

#[test]
fn shuold_build_a_vec_of_types() {
    let args: Vec<std::ffi::OsString> = vec!["".into()];
    let config = Config::parse_from(args);
    assert_eq!(vec![MyType::A, MyType::B, MyType::A], config.values);
}

#[test]
fn shuold_build_a_vec_of_types_from_default_value() {
    let args: Vec<std::ffi::OsString> = vec!["".into(), "--values=a,b".into()];
    let config = Config::parse_from(args);
    assert_eq!(vec![MyType::A, MyType::B], config.values);
}

Steps to reproduce the bug with the above code

Executes the two tests provided in the minimal reproducible code; they both fail in 3.1 but pass in 3.0

Actual Behaviour

A validation error is thrown when the configuration is built.

Expected Behaviour

The configuration is valid in 3.0 and should be valid in 3.1 too.

Additional Context

No response

Debug Output

No response

@ufoscout ufoscout added the C-bug Category: Updating dependencies label Feb 26, 2022
@epage epage added A-builder Area: Builder API E-easy Call for participation: Experience needed to fix: Easy / not much labels Feb 26, 2022
@epage
Copy link
Member

epage commented Feb 26, 2022

The error message

thread 'main' panicked at 'Argument values's default_value=a,b,a failed validation: ', /home/epage/src/personal/clap/src/build/debug_asserts.rs:690:21
note: run with RUST_BACKTRACE=1 environment variable to display a backtrace

This regression was introduced in #3423 because delimited values were not taken into account.

To fix this, we need to take delimited values into account and add tests for this case (ideally at the builder level).

@epage
Copy link
Member

epage commented Feb 28, 2022

v3.1.3 is now released with a fix for this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builder Area: Builder API C-bug Category: Updating dependencies E-easy Call for participation: Experience needed to fix: Easy / not much
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants