Skip to content

Commit

Permalink
fix(v4): Disallow leading dashes in long's
Browse files Browse the repository at this point in the history 
This is a step towards clap-rs#3309.  We want to make longs and long aliases
more consistent in how they handle leading dashes.  There is more
flexibility offered in not stripping and it matches the v3 short
behavior of only taking the non-dash form.  This starts the process by
disallowing it completely so people will catch problems with it and
remove their existing leading dashes.  In a subsequent breaking release
we can remove the debug assert and allow triple-leading dashes.
  • Loading branch information
@epage
epage committed May 4, 2022
1 parent 8bea44d commit d3e36b1
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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
View file
Original file line number Diff line number Diff line change
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 d3e36b1

Please sign in to comment.