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

Allow visible aliases for PossibleValue #4416

Open
2 tasks done
dbrgn opened this issue Oct 22, 2022 · 2 comments
Open
2 tasks done

Allow visible aliases for PossibleValue #4416

dbrgn opened this issue Oct 22, 2022 · 2 comments
Labels
A-builder Area: Builder API A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate

Comments

@dbrgn
Copy link

dbrgn commented Oct 22, 2022

Please complete the following tasks

Clap Version

4.0.18

Describe your use case

Tealdeer has a parameter -p, --platform <PLATFORM> to select the platform. For historic reasons, this list must include "osx", but should also include the alias "macos". This alias should not be hidden, but should be visible.

With Clap 3, this was implemented using FromStr:

#[derive(Debug, Eq, PartialEq, Copy, Clone, Serialize, Deserialize)]
#[serde(rename_all = "lowercase")]
#[allow(dead_code)]
pub enum PlatformType {
    Linux,
    OsX,
    SunOs,
    Windows,
    Android,
}

impl str::FromStr for PlatformType {
    type Err = anyhow::Error;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "linux" => Ok(Self::Linux),
            "osx" | "macos" => Ok(Self::OsX),
            "sunos" => Ok(Self::SunOs),
            "windows" => Ok(Self::Windows),
            "android" => Ok(Self::Android),
            other => Err(anyhow!(
                "Unknown OS: {}. Possible values: linux, macos, osx, sunos, windows, android",
                other
            )),
        }
    }
}
    /// Override the operating system
    #[clap(
        short = 'p',
        long = "platform",
        possible_values = ["linux", "macos", "windows", "sunos", "osx", "android"],
    )]
    pub platform: Option<PlatformType>,

With Clap 4, I tried to migrate to EnumValueParser:

impl clap::ValueEnum for PlatformType {
    fn value_variants<'a>() -> &'a [Self] {
        &[Self::Linux, Self::OsX, Self::Windows, Self::SunOs, Self::Android]
    }

    fn to_possible_value<'a>(&self) -> Option<clap::builder::PossibleValue> {
        match self {
            Self::Linux => Some(clap::builder::PossibleValue::new("linux")),
            Self::OsX => Some(clap::builder::PossibleValue::new("osx").alias("macos")),
            Self::Windows => Some(clap::builder::PossibleValue::new("windows")),
            Self::SunOs => Some(clap::builder::PossibleValue::new("sunos")),
            Self::Android => Some(clap::builder::PossibleValue::new("android")),
        }
    }
}

This works, but the alias is not visible in help output or error messages.

Describe the solution you'd like

Since Command has both .alias(...) and .visible_alias(...), would this be possible for PossibleValue as well?

Alternatives, if applicable

No response

Additional Context

No response

@dbrgn dbrgn added the C-enhancement Category: Raise on the bar on expectations label Oct 22, 2022
@dbrgn
Copy link
Author

dbrgn commented Oct 22, 2022

Note: This comment by @epage might be related / an alternative API to the suggested .visible_alias method:

Kind of like how clap switched from possible values being a &str to PossibleValue, I wonder if we should have an Alias<T> which has a hide method on it.

@epage epage added A-help Area: documentation, including docs.rs, readme, examples, etc... A-builder Area: Builder API E-medium Call for participation: Experience needed to fix: Medium / intermediate E-help-wanted Call for participation: Help is requested to fix this issue. labels Oct 24, 2022
@epage
Copy link
Member

epage commented Oct 24, 2022

I broke that comment out into #4421. I think I'd prefer not going that route for PossibleValue until we do it for everything as it'd be weird to have such a divergent API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-builder Area: Builder API A-help Area: documentation, including docs.rs, readme, examples, etc... C-enhancement Category: Raise on the bar on expectations E-help-wanted Call for participation: Help is requested to fix this issue. E-medium Call for participation: Experience needed to fix: Medium / intermediate
Projects
None yet
Development

No branches or pull requests

2 participants