clap icon indicating copy to clipboard operation
clap copied to clipboard

Allow visible aliases for PossibleValue

Open dbrgn opened this issue 3 years ago • 6 comments

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 avatar Oct 22 '22 20:10 dbrgn

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.

dbrgn avatar Oct 22 '22 20:10 dbrgn

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.

epage avatar Oct 24 '22 13:10 epage

The big question for me for this is how visible aliases for possible values should be rendered. In the short help, it feels weird to redundantly list options that map back to the same thing and would get weird nesting expressions into each other. With long help, we could have an [aliases: ...] after the item, I guess.

epage avatar Jun 23 '24 00:06 epage

I thought of two solutions for the short help:

  • Not rendering them in that case at all since the short help should be compact and it must not have all the information (Like we are not printing the description for value eighter)
  • Adding brackets next to each option without the word 'aliases' in it. Something like [possible values: foo-option (foo, f) , bar-option (b), no-alias-option]

We can make printing aliases in short description optional be adding a boolean or a flag for the function. Something like:

// Boolean option 
pub fn visible_alias(mut self, name: impl IntoResettable<Str>, short-help: bool) -> ...

// Flag option 
pub fn visible_alias(mut self, name: impl IntoResettable<Str>, flags: AliasFlags) -> ...
bitflags! {
    pub struct AliasFlags: u32 {
        const ShortHelp = 0b00000001;
       ...
    }
}

AmmarAbouZor avatar Aug 21 '24 07:08 AmmarAbouZor

We can make printing aliases in short description optional be adding a boolean or a flag for the function.

We're trying to keep minimum the configuration we provide, focusing on a consistent, quality out of box experience.

epage avatar Aug 21 '24 14:08 epage

@dbrgn you could probably achieve something similar to what you want (at least displaying them all - not more custom formatting) using something like this:

PossibleValuesParser.map() -> MapValueParser (docs.rs)

i.e. possible_values() returns all of the names + aliases and map_value() does the many-to-one alias mapping to your defined enum variants.

see: https://github.com/getgrit/gritql/pull/544/commits/a5887a7

use clap::builder::{MapValueParser, PossibleValuesParser, TypedValueParser};

#[derive(ValueEnum, Debug, Clone, Copy, Default, PartialEq, Eq, Hash, Serialize, Deserialize)]
#[clap(rename_all = "lower")]
#[serde(rename_all = "lowercase")]
pub enum PlatformType {
    ...
}

impl PlatformType {
    fn get_aliases(&self) -> &'static [&'static str] {
        ...
    }

    fn map_value(s: String) -> Self {
        ...
    }

    fn possible_values() -> Vec<&'static str> {
        ...
    }

    pub fn value_parser() -> MapValueParser<PossibleValuesParser, fn(String) -> Self> {
        PossibleValuesParser::new(Self::possible_values()).map(|s| Self::map_value(s))
    }

usage:

#[derive(Args, Clone, Debug, Serialize)]
pub struct YourArgs {
    #[clap(short = 'p', long = "platform", value_parser = PlatformType::value_parser())]
    pub platform: Option<PlatformType>,
}

Alex-ley-scrub avatar Oct 25 '24 18:10 Alex-ley-scrub

Hi, when I wrote this commit, I couldn't find a way to print the aliases from the ValueEnum with --help. So I hard-coded the aliases in the doc comments as you can see...:

    /// Standard X-25 CRC-16 [aliases: crc16_x25, 16]
    #[clap(alias = "crc16_x25", alias = "16")]

I don't like this, and I reckon this is originates from this issue.

If possible I'd like to get it done so that I can get rid of my hard-coded values in this other code. There seems to be unanswered design questions as to how to provide this feature?

theotchlx avatar Nov 25 '25 15:11 theotchlx

I think it's better to have an unstable (versioned?) implementation rather than no implementation.

eirnym avatar Nov 25 '25 18:11 eirnym

I think it's better to have an unstable (versioned?) implementation rather than no implementation.

Could you clarify your thoughts? Are you referring to an existing implementation or some design idea?

theotchlx avatar Nov 26 '25 07:11 theotchlx

The main problem I see in this PR is not that implementation of this PR would help people, but how data should be presented to a user. My statement is that any implementation would be better than no implementation.

There always be people, who don't agree on specific output. For example, we all know from the past, that major version may break how data is configured and presented, this is why I still see multiple tools using clap 3.x rather than the latest version.

My additional proposal is to have presentation to be always feature-gated. And if there would be mechanism to provide representation of a specific feature, it would be even better. In this way even new representation would be placed, most people would use newest library version. rather than froze the version.

eirnym avatar Nov 26 '25 15:11 eirnym