From 58436e30e1e1afa8f526d97635676dbbb731c9ec Mon Sep 17 00:00:00 2001 From: Alex Crichton 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, _>>()?; - 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 = ["-i", "true", "-p", "1.1"] + let args: Vec = ["--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 = ["-i", "true", "-p", "1.1"] + let args: Vec = ["--ignore=true", "-p1.1"] .iter() .map(|x| x.to_string()) .collect();