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

conflicts_with doesn't work if the long name is not the same as the field name #459

Closed
HalfVoxel opened this issue Jan 22, 2021 · 5 comments

Comments

@HalfVoxel
Copy link

HalfVoxel commented Jan 22, 2021

This does not work. Structopt freely allows me to specify both the name and cid argument. There are no compile errors of any kind:

#[derive(Debug, StructOpt, Clone)]
pub struct Opts {
    #[structopt(short = "n", long = "name")]
    pub module_name: Option<String>,

    #[structopt(short = "c", long = "cid", conflicts_with = "module_name")]
    pub module_cid: Option<Cid>,
}

Neither does this:

#[derive(Debug, StructOpt, Clone)]
pub struct Opts {
    #[structopt(short = "n", long = "name")]
    pub module_name: Option<String>,

    #[structopt(short = "c", long = "cid", conflicts_with = "name")]
    pub module_cid: Option<Cid>,
}

However, this does work. Structopt correctly prevents me from using the cid and name arguments together:

#[derive(Debug, StructOpt, Clone)]
pub struct Opts {
    #[structopt(short = "n", long = "name")]
    pub name: Option<String>,

    #[structopt(short = "c", long = "cid", conflicts_with = "name")]
    pub cid: Option<Cid>,
}

This seems like a bug. conflicts_with should not assume that the field name is the same as the long name.

Using structopt 0.3.29 with rustc 1.49.0 on ubuntu.

@TeXitoi
Copy link
Owner

TeXitoi commented Jan 22, 2021

https://docs.rs/structopt/0.3.21/structopt/#specifying-argument-types

Name is translated to kebab-case by defaullt, so the answer is:

#[derive(Debug, StructOpt, Clone)]
pub struct Opts {
    #[structopt(short = "n", long = "name")]
    pub module_name: Option<String>,

    #[structopt(short = "c", long = "cid", conflicts_with = "module-name")]
    pub module_cid: Option<Cid>,
}

@HalfVoxel
Copy link
Author

Ah, I see. It would be kinda helpful with some validation for this.
Imo it is not intuitive to have to use 'module-name' since that is never used anywhere (not even in the cli because we specified a custom long name for it).

@TeXitoi
Copy link
Owner

TeXitoi commented Jan 22, 2021

We can't really validate, as structopt doesn't know what conflicts_with is, it just generate codes that might be defined in clap.

This kebab-case thing allows to have default that are coherent with shell script conventions, but it also has its drawbacks as you pointed here.

@HalfVoxel
Copy link
Author

It can validate at runtime though? At some point it needs to check if it actually conflicts with anything and there it can presumably also see that "oh, I'm trying to check for a field that doesn't even exist". I am however not familiar with the internals of structopt so maybe this is harder than it seems.

@TeXitoi
Copy link
Owner

TeXitoi commented Jan 22, 2021

It would be a clap feature, structopt just generate .conflicts_with("name") that is defined in clap.

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