From 58436e30e1e1afa8f526d97635676dbbb731c9ec Mon Sep 17 00:00:00 2001
From: Alex Crichton <alex@alexcrichton.com>
Date: Mon, 19 Aug 2019 08:02:40 -0700
Subject: [PATCH] Revert #81, #82, #83
This commit reverts three recent PRs to the `getopts` crate. One of the
main consumers of `getopts` is `rustc`, which isn't allowed to have
breaking changes. In #82, however, a breaking change was landed. It
looks like #83 builds on this change, and while #81 seems unrelated the
diffs were somewhat tangled.
---
src/lib.rs | 82 ++++++++++++++++++++++--------------------------
src/tests/mod.rs | 39 +++++++++++++----------
2 files changed, 60 insertions(+), 61 deletions(-)
diff --git a/src/lib.rs b/src/lib.rs
index 9a9d5d20..925c8b5e 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -364,7 +364,7 @@ impl Options {
.ok_or_else(|| Fail::UnrecognizedOption(format!("{:?}", i.as_ref())))
.map(|s| s.to_owned())
}).collect::<::std::result::Result<Vec<_>, _>>()?;
- let mut args = args.into_iter().peekable();
+ let mut args = args.into_iter();
let mut arg_pos = 0;
while let Some(cur) = args.next() {
if !is_arg(&cur) {
@@ -380,9 +380,8 @@ impl Options {
free.extend(args);
break;
} else {
- let mut names;
+ let mut name = None;
let mut i_arg = None;
- let mut was_long = true;
if cur.as_bytes()[1] == b'-' || self.long_only {
let tail = if cur.as_bytes()[1] == b'-' {
&cur[2..]
@@ -391,81 +390,71 @@ impl Options {
&cur[1..]
};
let mut parts = tail.splitn(2, '=');
- names = vec![Name::from_str(parts.next().unwrap())];
+ name = Some(Name::from_str(parts.next().unwrap()));
if let Some(rest) = parts.next() {
i_arg = Some(rest.to_string());
}
} else {
- was_long = false;
- names = Vec::new();
for (j, ch) in cur.char_indices().skip(1) {
let opt = Short(ch);
- /* In a series of potential options (eg. -aheJ), if we
- see one which takes an argument, we assume all
- subsequent characters make up the argument. This
- allows options such as -L/usr/local/lib/foo to be
- interpreted correctly
- */
-
let opt_id = match find_opt(&opts, &opt) {
Some(id) => id,
None => return Err(UnrecognizedOption(opt.to_string())),
};
- names.push(opt);
-
+ // In a series of potential options (eg. -aheJ), if we
+ // see one which takes an argument, we assume all
+ // subsequent characters make up the argument. This
+ // allows options such as -L/usr/local/lib/foo to be
+ // interpreted correctly
let arg_follows = match opts[opt_id].hasarg {
Yes | Maybe => true,
No => false,
};
if arg_follows {
+ name = Some(opt);
let next = j + ch.len_utf8();
if next < cur.len() {
i_arg = Some(cur[next..].to_string());
break;
}
+ } else {
+ vals[opt_id].push((arg_pos, Given));
}
}
}
- let mut name_pos = 0;
- for nm in names.iter() {
- name_pos += 1;
- let optid = match find_opt(&opts, &nm) {
+ if let Some(nm) = name {
+ let opt_id = match find_opt(&opts, &nm) {
Some(id) => id,
None => return Err(UnrecognizedOption(nm.to_string())),
};
- match opts[optid].hasarg {
+ match opts[opt_id].hasarg {
No => {
- if name_pos == names.len() && i_arg.is_some() {
+ if i_arg.is_some() {
return Err(UnexpectedArgument(nm.to_string()));
}
- vals[optid].push((arg_pos, Given));
+ vals[opt_id].push((arg_pos, Given));
}
Maybe => {
- // Note that here we do not handle `--arg value`.
- // This matches GNU getopt behavior; but also
- // makes sense, because if this were accepted,
- // then users could only write a "Maybe" long
- // option at the end of the arguments when
- // FloatingFrees is in use.
+ // Note that here we do not handle `--arg value` or
+ // `-a value`. This matches GNU getopt behavior; but
+ // also makes sense, because if this were accepted,
+ // then users could only write a "Maybe" option at
+ // the end of the arguments when FloatingFrees is in
+ // use.
if let Some(i_arg) = i_arg.take() {
- vals[optid].push((arg_pos, Val(i_arg)));
- } else if was_long
- || name_pos < names.len()
- || args.peek().map_or(true, |n| is_arg(&n))
- {
- vals[optid].push((arg_pos, Given));
+ vals[opt_id].push((arg_pos, Val(i_arg)));
} else {
- vals[optid].push((arg_pos, Val(args.next().unwrap())));
+ vals[opt_id].push((arg_pos, Given));
}
}
Yes => {
if let Some(i_arg) = i_arg.take() {
- vals[optid].push((arg_pos, Val(i_arg)));
+ vals[opt_id].push((arg_pos, Val(i_arg)));
} else if let Some(n) = args.next() {
- vals[optid].push((arg_pos, Val(n)));
+ vals[opt_id].push((arg_pos, Val(n)));
} else {
return Err(ArgumentMissing(nm.to_string()));
}
@@ -551,10 +540,6 @@ impl Options {
row.push_str(&short_name);
if long_name.width() > 0 {
row.push_str(", ");
- } else {
- // Only a single space here, so that any
- // argument is printed in the correct spot.
- row.push(' ');
}
}
// FIXME: refer issue #7.
@@ -567,16 +552,21 @@ impl Options {
_ => {
row.push_str(if self.long_only { "-" } else { "--" });
row.push_str(&long_name);
- row.push(' ');
}
}
// arg
match hasarg {
No => {}
- Yes => row.push_str(&hint),
+ Yes => {
+ row.push(' ');
+ row.push_str(&hint);
+ }
Maybe => {
row.push('[');
+ if !long_name.is_empty() {
+ row.push('=');
+ }
row.push_str(&hint);
row.push(']');
}
@@ -979,9 +969,13 @@ fn format_option(opt: &OptGroup) -> String {
}
if opt.hasarg != No {
- line.push(' ');
if opt.hasarg == Maybe {
line.push('[');
+ if opt.short_name.is_empty() {
+ line.push('=');
+ }
+ } else {
+ line.push(' ');
}
line.push_str(&opt.hint);
if opt.hasarg == Maybe {
diff --git a/src/tests/mod.rs b/src/tests/mod.rs
index c61beaa5..40bf0d2a 100644
--- a/src/tests/mod.rs
+++ b/src/tests/mod.rs
@@ -379,8 +379,8 @@ fn test_optflagopt() {
let short_args = vec!["-t".to_string(), "x".to_string()];
match opts.parse(&short_args) {
Ok(ref m) => {
- assert_eq!(m.opt_str("t").unwrap(), "x");
- assert_eq!(m.opt_str("test").unwrap(), "x");
+ assert_eq!(m.opt_str("t"), None);
+ assert_eq!(m.opt_str("test"), None);
}
_ => panic!(),
}
@@ -763,6 +763,8 @@ fn test_usage() {
opts.optopt("a", "012345678901234567890123456789", "Desc", "VAL");
opts.optflag("k", "kiwi", "Desc");
opts.optflagopt("p", "", "Desc", "VAL");
+ opts.optflagopt("", "pea", "Desc", "VAL");
+ opts.optflagopt("c", "cherry", "Desc", "VAL");
opts.optmulti("l", "", "Desc", "VAL");
opts.optflag("", "starfruit", "Starfruit");
@@ -773,7 +775,9 @@ Options:
-a, --012345678901234567890123456789 VAL
Desc
-k, --kiwi Desc
- -p [VAL] Desc
+ -p[VAL] Desc
+ --pea[=VAL] Desc
+ -c, --cherry[=VAL] Desc
-l VAL Desc
--starfruit Starfruit
";
@@ -820,7 +824,7 @@ Options:
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -851,7 +855,7 @@ Options:
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -882,7 +886,7 @@ Options:
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -909,7 +913,7 @@ Options:
-c, --brûlée brûlée quite long description
-k, --kiwi€ kiwi description
-o, --orange‹ orange description
- -r, --raspberry-but-making-this-option-way-too-long\u{0020}
+ -r, --raspberry-but-making-this-option-way-too-long
raspberry description is also quite long indeed longer
than every other piece of text we might encounter here
and thus will be automatically broken up
@@ -919,7 +923,7 @@ Options:
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -934,13 +938,13 @@ fn test_usage_short_only() {
Options:
-k VAL Kiwi
-s Starfruit
- -a [TYPE] Apple
+ -a[TYPE] Apple
";
let usage = opts.usage("Usage: fruits");
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -955,13 +959,13 @@ fn test_usage_long_only() {
Options:
--kiwi VAL Kiwi
--starfruit Starfruit
- --apple [TYPE] Apple
+ --apple[=TYPE] Apple
";
let usage = opts.usage("Usage: fruits");
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -971,9 +975,10 @@ fn test_short_usage() {
opts.optopt("a", "012345678901234567890123456789", "Desc", "VAL");
opts.optflag("k", "kiwi", "Desc");
opts.optflagopt("p", "", "Desc", "VAL");
- opts.optmulti("l", "", "Desc", "VAL");
+ opts.optflagopt("", "pea", "Desc", "VAL");
+ opts.optflagopt("c", "cherry", "Desc", "VAL");
- let expected = "Usage: fruits -b VAL [-a VAL] [-k] [-p [VAL]] [-l VAL]..".to_string();
+ let expected = "Usage: fruits -b VAL [-a VAL] [-k] [-p[VAL]] [--pea[=VAL]] [-c[VAL]]".to_string();
let generated_usage = opts.short_usage("fruits");
debug!("expected: <<{}>>", expected);
@@ -1026,7 +1031,7 @@ Options:
debug!("expected: <<{}>>", expected);
debug!("generated: <<{}>>", usage);
- assert!(usage == expected)
+ assert_eq!(usage, expected)
}
#[test]
@@ -1148,7 +1153,7 @@ fn test_opt_get() {
opts.optflagopt("p", "percent", "Description", "0.0 .. 10.0");
opts.long_only(false);
- let args: Vec<String> = ["-i", "true", "-p", "1.1"]
+ let args: Vec<String> = ["--ignore=true", "-p1.1"]
.iter()
.map(|x| x.to_string())
.collect();
@@ -1173,7 +1178,7 @@ fn test_opt_get_default() {
opts.optflagopt("p", "percent", "Description", "0.0 .. 10.0");
opts.long_only(false);
- let args: Vec<String> = ["-i", "true", "-p", "1.1"]
+ let args: Vec<String> = ["--ignore=true", "-p1.1"]
.iter()
.map(|x| x.to_string())
.collect();