Skip to content

Commit

Permalink
Merge pull request #3691 from epage/dash
Browse files Browse the repository at this point in the history
fix(v4): Disallow leading dashes in long's
  • Loading branch information
epage committed May 4, 2022
2 parents 8bea44d + d3e36b1 commit fff1b11
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 24 deletions.
4 changes: 2 additions & 2 deletions clap_complete/tests/common.rs
Expand Up @@ -48,7 +48,7 @@ pub fn special_commands_command(name: &'static str) -> clap::Command<'static> {
.about("tests other things")
.arg(
clap::Arg::new("config")
.long("--config")
.long("config")
.hide(true)
.takes_value(true)
.require_equals(true)
Expand Down Expand Up @@ -139,7 +139,7 @@ pub fn sub_subcommands_command(name: &'static str) -> clap::Command<'static> {
.subcommand(
clap::Command::new("sub_cmd").about("sub-subcommand").arg(
clap::Arg::new("config")
.long("--config")
.long("config")
.takes_value(true)
.possible_values([clap::PossibleValue::new("Lest quotes aren't escaped.")])
.help("the other case to test"),
Expand Down
4 changes: 2 additions & 2 deletions clap_complete_fig/tests/common.rs
Expand Up @@ -48,7 +48,7 @@ pub fn special_commands_command(name: &'static str) -> clap::Command<'static> {
.about("tests other things")
.arg(
clap::Arg::new("config")
.long("--config")
.long("config")
.hide(true)
.takes_value(true)
.require_equals(true)
Expand Down Expand Up @@ -139,7 +139,7 @@ pub fn sub_subcommands_command(name: &'static str) -> clap::Command<'static> {
.subcommand(
clap::Command::new("sub_cmd").about("sub-subcommand").arg(
clap::Arg::new("config")
.long("--config")
.long("config")
.takes_value(true)
.possible_values([clap::PossibleValue::new("Lest quotes aren't escaped.")])
.help("the other case to test"),
Expand Down
8 changes: 4 additions & 4 deletions clap_mangen/tests/common.rs
Expand Up @@ -45,7 +45,7 @@ pub fn special_commands_command(name: &'static str) -> clap::Command<'static> {
.about("tests other things")
.arg(
clap::Arg::new("config")
.long("--config")
.long("config")
.takes_value(true)
.help("the other case to test"),
),
Expand Down Expand Up @@ -128,7 +128,7 @@ pub fn sub_subcommands_command(name: &'static str) -> clap::Command<'static> {
.subcommand(
clap::Command::new("sub_cmd").about("sub-subcommand").arg(
clap::Arg::new("config")
.long("--config")
.long("config")
.takes_value(true)
.possible_values([clap::PossibleValue::new("Lest quotes aren't escaped.")])
.help("the other case to test"),
Expand Down Expand Up @@ -222,10 +222,10 @@ pub fn value_hint_command(name: &'static str) -> clap::Command<'static> {

pub fn hidden_option_command(name: &'static str) -> clap::Command<'static> {
clap::Command::new(name)
.arg(clap::Arg::new("config").long("--config").takes_value(true))
.arg(clap::Arg::new("config").long("config").takes_value(true))
.arg(
clap::Arg::new("no-config")
.long("--no-config")
.long("no-config")
.hide(true)
.overrides_with("config"),
)
Expand Down
2 changes: 1 addition & 1 deletion src/bin/stdio-fixture.rs
Expand Up @@ -6,7 +6,7 @@ fn main() {
.subcommand(clap::Command::new("more"))
.arg(
clap::Arg::new("verbose")
.long("--verbose")
.long("verbose")
.help("log")
.long_help("more log"),
);
Expand Down
13 changes: 10 additions & 3 deletions src/build/arg.rs
Expand Up @@ -200,7 +200,14 @@ impl<'help> Arg<'help> {
#[inline]
#[must_use]
pub fn long(mut self, l: &'help str) -> Self {
self.long = Some(l.trim_start_matches(|c| c == '-'));
#[cfg(feature = "unstable-v4")]
{
self.long = Some(l);
}
#[cfg(not(feature = "unstable-v4"))]
{
self.long = Some(l.trim_start_matches(|c| c == '-'));
}
self
}

Expand Down Expand Up @@ -1819,7 +1826,7 @@ impl<'help> Arg<'help> {
/// # use clap::{Command, Arg};
/// let m = Command::new("pv")
/// .arg(Arg::new("option")
/// .long("--option")
/// .long("option")
/// .takes_value(true)
/// .ignore_case(true)
/// .possible_value("test123"))
Expand All @@ -1837,7 +1844,7 @@ impl<'help> Arg<'help> {
/// let m = Command::new("pv")
/// .arg(Arg::new("option")
/// .short('o')
/// .long("--option")
/// .long("option")
/// .takes_value(true)
/// .ignore_case(true)
/// .multiple_values(true)
Expand Down
9 changes: 8 additions & 1 deletion src/build/command.rs
Expand Up @@ -2263,7 +2263,14 @@ impl<'help> App<'help> {
/// [`Arg::long`]: Arg::long()
#[must_use]
pub fn long_flag(mut self, long: &'help str) -> Self {
self.long_flag = Some(long.trim_start_matches(|c| c == '-'));
#[cfg(feature = "unstable-v4")]
{
self.long_flag = Some(long);
}
#[cfg(not(feature = "unstable-v4"))]
{
self.long_flag = Some(long.trim_start_matches(|c| c == '-'));
}
self
}

Expand Down
8 changes: 8 additions & 0 deletions src/build/debug_asserts.rs
Expand Up @@ -45,6 +45,10 @@ pub(crate) fn assert_app(cmd: &Command) {
}

if let Some(l) = sc.get_long_flag().as_ref() {
#[cfg(feature = "unstable-v4")]
{
assert!(!l.starts_with('-'), "Command {}: long_flag {:?} must not start with a `-`, that will be handled by the parser", sc.get_name(), l);
}
long_flags.push(Flag::Command(format!("--{}", l), sc.get_name()));
}

Expand Down Expand Up @@ -75,6 +79,10 @@ pub(crate) fn assert_app(cmd: &Command) {
}

if let Some(l) = arg.long.as_ref() {
#[cfg(feature = "unstable-v4")]
{
assert!(!l.starts_with('-'), "Argument {}: long {:?} must not start with a `-`, that will be handled by the parser", arg.name, l);
}
long_flags.push(Flag::Arg(format!("--{}", l), &*arg.name));
}

Expand Down
17 changes: 17 additions & 0 deletions tests/builder/flags.rs
Expand Up @@ -179,3 +179,20 @@ For more information try --help

utils::assert_output(cmd, "test -----", MULTIPLE_DASHES, true);
}

#[test]
#[cfg(not(feature = "unstable-v4"))]
fn leading_dash_stripped() {
let cmd = Command::new("mycat").arg(Arg::new("filename").long("--filename"));
let matches = cmd.try_get_matches_from(["mycat", "--filename"]).unwrap();
assert!(matches.is_present("filename"));
}

#[test]
#[cfg(feature = "unstable-v4")]
#[cfg(debug_assertions)]
#[should_panic = "Argument filename: long \"--filename\" must not start with a `-`, that will be handled by the parser"]
fn leading_dash_stripped() {
let cmd = Command::new("mycat").arg(Arg::new("filename").long("--filename"));
cmd.debug_assert();
}
20 changes: 10 additions & 10 deletions tests/builder/possible_values.rs
Expand Up @@ -134,7 +134,7 @@ fn possible_values_of_option() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123"),
)
Expand All @@ -153,7 +153,7 @@ fn possible_values_of_option_fail() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123"),
)
Expand All @@ -169,7 +169,7 @@ fn possible_values_of_option_multiple() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123")
.possible_value("test321")
Expand All @@ -193,7 +193,7 @@ fn possible_values_of_option_multiple_fail() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123")
.possible_value("test321")
Expand Down Expand Up @@ -298,7 +298,7 @@ fn alias() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value(PossibleValue::new("test123").alias("123"))
.possible_value("test321")
Expand All @@ -316,7 +316,7 @@ fn aliases() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value(PossibleValue::new("test123").aliases(["1", "2", "3"]))
.possible_value("test321")
Expand All @@ -334,7 +334,7 @@ fn ignore_case() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123")
.possible_value("test321")
Expand All @@ -356,7 +356,7 @@ fn ignore_case_fail() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123")
.possible_value("test321"),
Expand All @@ -373,7 +373,7 @@ fn ignore_case_multiple() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123")
.possible_value("test321")
Expand All @@ -395,7 +395,7 @@ fn ignore_case_multiple_fail() {
.arg(
Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("test123")
.possible_value("test321")
Expand Down
2 changes: 1 addition & 1 deletion tests/builder/unicode.rs
Expand Up @@ -6,7 +6,7 @@ fn possible_values_ignore_case() {
.arg(
clap::Arg::new("option")
.short('o')
.long("--option")
.long("option")
.takes_value(true)
.possible_value("ä")
.ignore_case(true),
Expand Down

0 comments on commit fff1b11

Please sign in to comment.