Error message should reference the short not long when short is used as arg

[polarathene]

Rust Version

1.25

Affected Version of clap

2.31.2

Expected Behavior Summary

Error should reference the short when used, not the long. Or at least also reference the short.

./target/debug/test -g not_a_valid_value
error: 'not_a_valid_value' isn't a valid value for '-g <NAME>'
        [possible values: opengl, vulkan]

or

./target/debug/test -g not_a_valid_value
error: 'not_a_valid_value' isn't a valid value for '-g / --force-renderer <NAME>'
        [possible values: opengl, vulkan]

Actual Behavior Summary

If a long is included, the error always references the long even when short is used for args.

./target/debug/test -g not_a_valid_value
error: 'not_a_valid_value' isn't a valid value for '--force-renderer <NAME>'
        [possible values: opengl, vulkan]

Sample Code or Link to Sample Code

extern crate clap;
use clap::{App,Arg};

fn main() {
    let matches = App::new("myapp")
        .arg(Arg::with_name("NAME")
            .takes_value(true)
            .short("g")
            .long("force-renderer")
            .possible_values(&["opengl", "vulkan"])
        )
        .get_matches();
}

[kbknapp]

Agreed, however this one is slightly harder to fix than one would think as the arguments aren't stored as they're used, and clap does all error parsing at the end, so actually doens't know which version of the argument was used.

This isn't un-fixable, just harder as clap would need to keep track of which version was used (long/short). Also there are strange edge cases when someone use both version for flags/options that allow multiple values. Consider -o, --option <value>... style argument. If used like $ prog -o value1 --option value2 and there's an error should clap report the short or long? If one says, "whichever version was used for that particular error", then clap needs to also track which version was used with each value.

So again, it's not undoable, it's just hard 😜 Having said all this, I'll leave this open, but only as a very low priority. I need to get 3.x out sometime in the next few months.

[epage]

We'd have to track this information per occurrence and track which occurrence we are validating at all times. The extra cost of us tracking this just for the error message does not seem worth it in terms of binary size, compile times, and memory usage.

In a way, the current behavior is better in that we clarify what the meaning of the short. By using a short, the user should be able to pick up on which of their arguments is being complained about though I could see it being nice to have rustc like error showing it in the line (if replace, defaults, and envs didn't cause problems)

As such, I lean towards closing this but we can re-open if people make the case for it.