diff --git a/src/options/filter.rs b/src/options/filter.rs index d961368..6d4b2f5 100644 --- a/src/options/filter.rs +++ b/src/options/filter.rs @@ -21,10 +21,6 @@ impl FileFilter { } } -const SORTS: &[&str] = &[ "name", "Name", "size", "extension", - "Extension", "modified", "accessed", - "created", "inode", "type", "none" ]; - impl SortField { /// Determines which sort field to use based on the `--sort` argument. @@ -75,7 +71,7 @@ impl SortField { Ok(SortField::Unsorted) } else { - Err(Misfire::bad_argument(&flags::SORT, word, SORTS)) + Err(Misfire::BadArgument(&flags::SORT, word.into())) } } } @@ -185,12 +181,6 @@ mod test { use options::flags; use options::parser::Flag; - pub fn os(input: &'static str) -> OsString { - let mut os = OsString::new(); - os.push(input); - os - } - macro_rules! test { ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => { #[test] @@ -226,7 +216,7 @@ mod test { test!(age: SortField <- ["-sage"]; Both => Ok(SortField::ModifiedAge)); // Errors - test!(error: SortField <- ["--sort=colour"]; Both => Err(Misfire::bad_argument(&flags::SORT, &os("colour"), super::SORTS))); + test!(error: SortField <- ["--sort=colour"]; Both => Err(Misfire::BadArgument(&flags::SORT, OsString::from("colour")))); // Overriding test!(overridden: SortField <- ["--sort=cr", "--sort", "mod"]; Last => Ok(SortField::ModifiedDate)); diff --git a/src/options/flags.rs b/src/options/flags.rs index 8f24dab..3f8eb20 100644 --- a/src/options/flags.rs +++ b/src/options/flags.rs @@ -1,4 +1,4 @@ -use options::parser::{Arg, Args, TakesValue}; +use options::parser::{Arg, Args, Values, TakesValue}; // exa options @@ -14,8 +14,9 @@ pub static RECURSE: Arg = Arg { short: Some(b'R'), long: "recurse", takes_valu pub static TREE: Arg = Arg { short: Some(b'T'), long: "tree", takes_value: TakesValue::Forbidden }; pub static CLASSIFY: Arg = Arg { short: Some(b'F'), long: "classify", takes_value: TakesValue::Forbidden }; -pub static COLOR: Arg = Arg { short: None, long: "color", takes_value: TakesValue::Necessary }; -pub static COLOUR: Arg = Arg { short: None, long: "colour", takes_value: TakesValue::Necessary }; +pub static COLOR: Arg = Arg { short: None, long: "color", takes_value: TakesValue::Necessary(Some(COLOURS)) }; +pub static COLOUR: Arg = Arg { short: None, long: "colour", takes_value: TakesValue::Necessary(Some(COLOURS)) }; +const COLOURS: &[&str] = &["always", "auto", "never"]; pub static COLOR_SCALE: Arg = Arg { short: None, long: "color-scale", takes_value: TakesValue::Forbidden }; pub static COLOUR_SCALE: Arg = Arg { short: None, long: "colour-scale", takes_value: TakesValue::Forbidden }; @@ -23,11 +24,14 @@ pub static COLOUR_SCALE: Arg = Arg { short: None, long: "colour-scale", takes_va // filtering and sorting options pub static ALL: Arg = Arg { short: Some(b'a'), long: "all", takes_value: TakesValue::Forbidden }; pub static LIST_DIRS: Arg = Arg { short: Some(b'd'), long: "list-dirs", takes_value: TakesValue::Forbidden }; -pub static LEVEL: Arg = Arg { short: Some(b'L'), long: "level", takes_value: TakesValue::Necessary }; +pub static LEVEL: Arg = Arg { short: Some(b'L'), long: "level", takes_value: TakesValue::Necessary(None) }; pub static REVERSE: Arg = Arg { short: Some(b'r'), long: "reverse", takes_value: TakesValue::Forbidden }; -pub static SORT: Arg = Arg { short: Some(b's'), long: "sort", takes_value: TakesValue::Necessary }; -pub static IGNORE_GLOB: Arg = Arg { short: Some(b'I'), long: "ignore-glob", takes_value: TakesValue::Necessary }; +pub static SORT: Arg = Arg { short: Some(b's'), long: "sort", takes_value: TakesValue::Necessary(Some(SORTS)) }; +pub static IGNORE_GLOB: Arg = Arg { short: Some(b'I'), long: "ignore-glob", takes_value: TakesValue::Necessary(None) }; pub static DIRS_FIRST: Arg = Arg { short: None, long: "group-directories-first", takes_value: TakesValue::Forbidden }; +const SORTS: Values = &[ "name", "Name", "size", "extension", + "Extension", "modified", "accessed", + "created", "inode", "type", "none" ]; // display options pub static BINARY: Arg = Arg { short: Some(b'b'), long: "binary", takes_value: TakesValue::Forbidden }; @@ -38,10 +42,12 @@ pub static INODE: Arg = Arg { short: Some(b'i'), long: "inode", takes_ pub static LINKS: Arg = Arg { short: Some(b'H'), long: "links", takes_value: TakesValue::Forbidden }; pub static MODIFIED: Arg = Arg { short: Some(b'm'), long: "modified", takes_value: TakesValue::Forbidden }; pub static BLOCKS: Arg = Arg { short: Some(b'S'), long: "blocks", takes_value: TakesValue::Forbidden }; -pub static TIME: Arg = Arg { short: Some(b't'), long: "time", takes_value: TakesValue::Necessary }; +pub static TIME: Arg = Arg { short: Some(b't'), long: "time", takes_value: TakesValue::Necessary(Some(TIMES)) }; pub static ACCESSED: Arg = Arg { short: Some(b'u'), long: "accessed", takes_value: TakesValue::Forbidden }; pub static CREATED: Arg = Arg { short: Some(b'U'), long: "created", takes_value: TakesValue::Forbidden }; -pub static TIME_STYLE: Arg = Arg { short: None, long: "time-style", takes_value: TakesValue::Necessary }; +pub static TIME_STYLE: Arg = Arg { short: None, long: "time-style", takes_value: TakesValue::Necessary(Some(TIME_STYLES)) }; +const TIMES: Values = &["modified", "accessed", "created"]; +const TIME_STYLES: Values = &["default", "long-iso", "full-iso", "iso"]; // optional feature options pub static GIT: Arg = Arg { short: None, long: "git", takes_value: TakesValue::Forbidden }; diff --git a/src/options/misfire.rs b/src/options/misfire.rs index b2a716e..58c1ef4 100644 --- a/src/options/misfire.rs +++ b/src/options/misfire.rs @@ -1,4 +1,4 @@ -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::fmt; use std::num::ParseIntError; @@ -17,7 +17,7 @@ pub enum Misfire { InvalidOptions(ParseError), /// The user supplied an illegal choice to an Argument. - BadArgument(&'static Arg, OsString, Choices), + BadArgument(&'static Arg, OsString), /// The user asked for help. This isn’t strictly an error, which is why /// this enum isn’t named Error! @@ -60,14 +60,6 @@ impl Misfire { _ => true, } } - - /// The Misfire that happens when an option gets given the wrong - /// argument. This has to use one of the `getopts` failure - /// variants--it’s meant to take just an option name, rather than an - /// option *and* an argument, but it works just as well. - pub fn bad_argument(option: &'static Arg, otherwise: &OsStr, legal: &'static [&'static str]) -> Misfire { - Misfire::BadArgument(option, otherwise.to_os_string(), Choices(legal)) - } } impl From for Misfire { @@ -78,10 +70,18 @@ impl From for Misfire { impl fmt::Display for Misfire { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + use options::parser::TakesValue; use self::Misfire::*; match *self { - BadArgument(ref a, ref b, ref c) => write!(f, "Option {} has no {:?} setting ({})", a, b, c), + BadArgument(ref arg, ref attempt) => { + if let TakesValue::Necessary(Some(values)) = arg.takes_value { + write!(f, "Option {} has no {:?} setting ({})", arg, attempt, Choices(values)) + } + else { + write!(f, "Option {} has no {:?} setting", arg, attempt) + } + }, InvalidOptions(ref e) => write!(f, "{}", e), Help(ref text) => write!(f, "{}", text), Version(ref version) => write!(f, "{}", version), @@ -103,17 +103,18 @@ impl fmt::Display for ParseError { use self::ParseError::*; match *self { - NeedsValue { ref flag } => write!(f, "Flag {} needs a value", flag), - ForbiddenValue { ref flag } => write!(f, "Flag {} cannot take a value", flag), - UnknownShortArgument { ref attempt } => write!(f, "Unknown argument -{}", *attempt as char), - UnknownArgument { ref attempt } => write!(f, "Unknown argument --{}", attempt.to_string_lossy()), + NeedsValue { ref flag, values: None } => write!(f, "Flag {} needs a value", flag), + NeedsValue { ref flag, values: Some(cs) } => write!(f, "Flag {} needs a value ({})", flag, Choices(cs)), + ForbiddenValue { ref flag } => write!(f, "Flag {} cannot take a value", flag), + UnknownShortArgument { ref attempt } => write!(f, "Unknown argument -{}", *attempt as char), + UnknownArgument { ref attempt } => write!(f, "Unknown argument --{}", attempt.to_string_lossy()), } } } impl Misfire { pub fn suggestion(&self) -> Option<&'static str> { - if let Misfire::BadArgument(ref time, ref r, ref _choices) = *self { + if let Misfire::BadArgument(ref time, ref r) = *self { if *time == &flags::TIME && r == "r" { return Some("To sort newest files first, try \"--sort modified\", or just \"-stime\""); } diff --git a/src/options/parser.rs b/src/options/parser.rs index 29bd709..f84f15e 100644 --- a/src/options/parser.rs +++ b/src/options/parser.rs @@ -43,6 +43,13 @@ pub type ShortArg = u8; /// which flag it was. pub type LongArg = &'static str; +/// A **list of values** that an option can have, to be displayed when the +/// user enters an invalid one or skips it. +/// +/// This is literally just help text, and won’t be used to validate a value to +/// see if it’s correct. +pub type Values = &'static [&'static str]; + /// A **flag** is either of the two argument types, because they have to /// be in the same array together. #[derive(PartialEq, Debug, Clone)] @@ -88,7 +95,9 @@ pub enum Strictness { pub enum TakesValue { /// This flag has to be followed by a value. - Necessary, + /// If there’s a fixed set of possible values, they can be printed out + /// with the error text. + Necessary(Option), /// This flag will throw an error if there’s a value after it. Forbidden, @@ -171,8 +180,8 @@ impl Args { let arg = self.lookup_long(before)?; let flag = Flag::Long(arg.long); match arg.takes_value { - Necessary => result_flags.push((flag, Some(after))), - Forbidden => return Err(ParseError::ForbiddenValue { flag }) + Necessary(_) => result_flags.push((flag, Some(after))), + Forbidden => return Err(ParseError::ForbiddenValue { flag }) } } @@ -182,13 +191,13 @@ impl Args { let arg = self.lookup_long(long_arg_name)?; let flag = Flag::Long(arg.long); match arg.takes_value { - Forbidden => result_flags.push((flag, None)), - Necessary => { + Forbidden => result_flags.push((flag, None)), + Necessary(values) => { if let Some(next_arg) = inputs.next() { result_flags.push((flag, Some(next_arg))); } else { - return Err(ParseError::NeedsValue { flag }) + return Err(ParseError::NeedsValue { flag, values }) } } } @@ -210,7 +219,7 @@ impl Args { // -abcdx= => error // // There’s no way to give two values in a cluster like this: - // it's an error if any of the first set of arguments actually + // it’s an error if any of the first set of arguments actually // takes a value. if let Some((before, after)) = split_on_equals(short_arg) { let (arg_with_value, other_args) = before.as_bytes().split_last().unwrap(); @@ -220,8 +229,8 @@ impl Args { let arg = self.lookup_short(*byte)?; let flag = Flag::Short(*byte); match arg.takes_value { - Forbidden => result_flags.push((flag, None)), - Necessary => return Err(ParseError::NeedsValue { flag }) + Forbidden => result_flags.push((flag, None)), + Necessary(values) => return Err(ParseError::NeedsValue { flag, values }) } } @@ -229,15 +238,15 @@ impl Args { let arg = self.lookup_short(*arg_with_value)?; let flag = Flag::Short(arg.short.unwrap()); match arg.takes_value { - Necessary => result_flags.push((flag, Some(after))), - Forbidden => return Err(ParseError::ForbiddenValue { flag }) + Necessary(_) => result_flags.push((flag, Some(after))), + Forbidden => return Err(ParseError::ForbiddenValue { flag }) } } // If there’s no equals, then every character is parsed as // its own short argument. However, if any of the arguments // takes a value, then the *rest* of the string is used as - // its value, and if there's no rest of the string, then it + // its value, and if there’s no rest of the string, then it // uses the next one in the iterator. // // -a => ‘a’ @@ -251,8 +260,8 @@ impl Args { let arg = self.lookup_short(*byte)?; let flag = Flag::Short(*byte); match arg.takes_value { - Forbidden => result_flags.push((flag, None)), - Necessary => { + Forbidden => result_flags.push((flag, None)), + Necessary(values) => { if index < bytes.len() - 1 { let remnants = &bytes[index+1 ..]; result_flags.push((flag, Some(OsStr::from_bytes(remnants)))); @@ -262,7 +271,7 @@ impl Args { result_flags.push((flag, Some(next_arg))); } else { - return Err(ParseError::NeedsValue { flag }) + return Err(ParseError::NeedsValue { flag, values }) } } } @@ -366,7 +375,7 @@ impl<'a> MatchedFlags<'a> { } /// Returns the value of the argument that matches the predicate if it - /// was specified, nothing if it wasn't, and an error in strict mode if + /// was specified, nothing if it wasn’t, and an error in strict mode if /// multiple arguments matched the predicate. /// /// It’s not possible to tell which flag the value belonged to from this. @@ -407,15 +416,15 @@ impl<'a> MatchedFlags<'a> { } -/// A problem with the user's input that meant it couldn't be parsed into a +/// A problem with the user’s input that meant it couldn’t be parsed into a /// coherent list of arguments. #[derive(PartialEq, Debug)] pub enum ParseError { /// A flag that has to take a value was not given one. - NeedsValue { flag: Flag }, + NeedsValue { flag: Flag, values: Option }, - /// A flag that can't take a value *was* given one. + /// A flag that can’t take a value *was* given one. ForbiddenValue { flag: Flag }, /// A short argument, either alone or in a cluster, was not @@ -543,10 +552,13 @@ mod parse_test { }; } + const SUGGESTIONS: Values = &[ "example" ]; + static TEST_ARGS: &[&Arg] = &[ &Arg { short: Some(b'l'), long: "long", takes_value: TakesValue::Forbidden }, &Arg { short: Some(b'v'), long: "verbose", takes_value: TakesValue::Forbidden }, - &Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary } + &Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary(None) }, + &Arg { short: Some(b't'), long: "type", takes_value: TakesValue::Necessary(Some(SUGGESTIONS)) } ]; @@ -569,10 +581,15 @@ mod parse_test { // Long args with values test!(bad_equals: ["--long=equals"] => error ForbiddenValue { flag: Flag::Long("long") }); - test!(no_arg: ["--count"] => error NeedsValue { flag: Flag::Long("count") }); + test!(no_arg: ["--count"] => error NeedsValue { flag: Flag::Long("count"), values: None }); test!(arg_equals: ["--count=4"] => frees: [], flags: [ (Flag::Long("count"), Some(OsStr::new("4"))) ]); test!(arg_then: ["--count", "4"] => frees: [], flags: [ (Flag::Long("count"), Some(OsStr::new("4"))) ]); + // Long args with values and suggestions + test!(no_arg_s: ["--type"] => error NeedsValue { flag: Flag::Long("type"), values: Some(SUGGESTIONS) }); + test!(arg_equals_s: ["--type=exa"] => frees: [], flags: [ (Flag::Long("type"), Some(OsStr::new("exa"))) ]); + test!(arg_then_s: ["--type", "exa"] => frees: [], flags: [ (Flag::Long("type"), Some(OsStr::new("exa"))) ]); + // Short args test!(short: ["-l"] => frees: [], flags: [ (Flag::Short(b'l'), None) ]); @@ -582,13 +599,19 @@ mod parse_test { // Short args with values test!(bad_short: ["-l=equals"] => error ForbiddenValue { flag: Flag::Short(b'l') }); - test!(short_none: ["-c"] => error NeedsValue { flag: Flag::Short(b'c') }); + test!(short_none: ["-c"] => error NeedsValue { flag: Flag::Short(b'c'), values: None }); test!(short_arg_eq: ["-c=4"] => frees: [], flags: [(Flag::Short(b'c'), Some(OsStr::new("4"))) ]); test!(short_arg_then: ["-c", "4"] => frees: [], flags: [(Flag::Short(b'c'), Some(OsStr::new("4"))) ]); test!(short_two_together: ["-lctwo"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]); test!(short_two_equals: ["-lc=two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]); test!(short_two_next: ["-lc", "two"] => frees: [], flags: [(Flag::Short(b'l'), None), (Flag::Short(b'c'), Some(OsStr::new("two"))) ]); + // Short args with values and suggestions + test!(short_none_s: ["-t"] => error NeedsValue { flag: Flag::Short(b't'), values: Some(SUGGESTIONS) }); + test!(short_two_together_s: ["-texa"] => frees: [], flags: [(Flag::Short(b't'), Some(OsStr::new("exa"))) ]); + test!(short_two_equals_s: ["-t=exa"] => frees: [], flags: [(Flag::Short(b't'), Some(OsStr::new("exa"))) ]); + test!(short_two_next_s: ["-t", "exa"] => frees: [], flags: [(Flag::Short(b't'), Some(OsStr::new("exa"))) ]); + // Unknown args test!(unknown_long: ["--quiet"] => error UnknownArgument { attempt: os("quiet") }); @@ -619,7 +642,7 @@ mod matches_test { } static VERBOSE: Arg = Arg { short: Some(b'v'), long: "verbose", takes_value: TakesValue::Forbidden }; - static COUNT: Arg = Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary }; + static COUNT: Arg = Arg { short: Some(b'c'), long: "count", takes_value: TakesValue::Necessary(None) }; test!(short_never: [], has VERBOSE => false); diff --git a/src/options/style.rs b/src/options/style.rs index 78970f0..470a912 100644 --- a/src/options/style.rs +++ b/src/options/style.rs @@ -34,7 +34,6 @@ impl Default for TerminalColours { } } -const COLOURS: &[&str] = &["always", "auto", "never"]; impl TerminalColours { @@ -56,7 +55,7 @@ impl TerminalColours { Ok(TerminalColours::Never) } else { - Err(Misfire::bad_argument(&flags::COLOR, word, COLOURS)) + Err(Misfire::BadArgument(&flags::COLOR, word.into())) } } } @@ -217,12 +216,6 @@ mod terminal_test { use options::test::parse_for_test; use options::test::Strictnesses::*; - pub fn os(input: &'static str) -> OsString { - let mut os = OsString::new(); - os.push(input); - os - } - static TEST_ARGS: &[&Arg] = &[ &flags::COLOR, &flags::COLOUR ]; macro_rules! test { @@ -260,8 +253,8 @@ mod terminal_test { test!(no_u_never: ["--color", "never"]; Both => Ok(TerminalColours::Never)); // Errors - test!(no_u_error: ["--color=upstream"]; Both => err Misfire::bad_argument(&flags::COLOR, &os("upstream"), super::COLOURS)); // the error is for --color - test!(u_error: ["--colour=lovers"]; Both => err Misfire::bad_argument(&flags::COLOR, &os("lovers"), super::COLOURS)); // and so is this one! + test!(no_u_error: ["--color=upstream"]; Both => err Misfire::BadArgument(&flags::COLOR, OsString::from("upstream"))); // the error is for --color + test!(u_error: ["--colour=lovers"]; Both => err Misfire::BadArgument(&flags::COLOR, OsString::from("lovers"))); // and so is this one! // Overriding test!(overridden_1: ["--colour=auto", "--colour=never"]; Last => Ok(TerminalColours::Never)); diff --git a/src/options/view.rs b/src/options/view.rs index deff114..bf7b6e9 100644 --- a/src/options/view.rs +++ b/src/options/view.rs @@ -248,8 +248,6 @@ impl SizeFormat { } -const TIME_STYLES: &[&str] = &["default", "long-iso", "full-iso", "iso"]; - impl TimeFormat { /// Determine how time should be formatted in timestamp columns. @@ -274,14 +272,12 @@ impl TimeFormat { Ok(TimeFormat::FullISO) } else { - Err(Misfire::bad_argument(&flags::TIME_STYLE, word, TIME_STYLES)) + Err(Misfire::BadArgument(&flags::TIME_STYLE, word.into())) } } } -static TIMES: &[&str] = &["modified", "accessed", "created"]; - impl TimeTypes { /// Determine which of a file’s time fields should be displayed for it @@ -320,7 +316,7 @@ impl TimeTypes { Ok(TimeTypes { accessed: false, modified: false, created: true }) } else { - Err(Misfire::bad_argument(&flags::TIME, word, TIMES)) + Err(Misfire::BadArgument(&flags::TIME, word.into())) } } else if modified || created || accessed { @@ -358,12 +354,6 @@ mod test { use options::test::parse_for_test; use options::test::Strictnesses::*; - pub fn os(input: &'static str) -> OsString { - let mut os = OsString::new(); - os.push(input); - os - } - static TEST_ARGS: &[&Arg] = &[ &flags::BINARY, &flags::BYTES, &flags::TIME_STYLE, &flags::TIME, &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED, &flags::HEADER, &flags::GROUP, &flags::INODE, &flags::GIT, @@ -461,7 +451,6 @@ mod test { mod time_formats { use super::*; - use std::ffi::OsStr; // These tests use pattern matching because TimeFormat doesn’t // implement PartialEq. @@ -483,7 +472,7 @@ mod test { test!(nevermore: TimeFormat <- ["--time-style", "long-iso", "--time-style=full-iso"]; Complain => err Misfire::Duplicate(Flag::Long("time-style"), Flag::Long("time-style"))); // Errors - test!(daily: TimeFormat <- ["--time-style=24-hour"]; Both => err Misfire::bad_argument(&flags::TIME_STYLE, OsStr::new("24-hour"), TIME_STYLES)); + test!(daily: TimeFormat <- ["--time-style=24-hour"]; Both => err Misfire::BadArgument(&flags::TIME_STYLE, OsString::from("24-hour"))); } @@ -515,8 +504,8 @@ mod test { test!(time_uu: TimeTypes <- ["-uU"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: true })); // Errors - test!(time_tea: TimeTypes <- ["--time=tea"]; Both => err Misfire::bad_argument(&flags::TIME, &os("tea"), super::TIMES)); - test!(time_ea: TimeTypes <- ["-tea"]; Both => err Misfire::bad_argument(&flags::TIME, &os("ea"), super::TIMES)); + test!(time_tea: TimeTypes <- ["--time=tea"]; Both => err Misfire::BadArgument(&flags::TIME, OsString::from("tea"))); + test!(time_ea: TimeTypes <- ["-tea"]; Both => err Misfire::BadArgument(&flags::TIME, OsString::from("ea"))); // Overriding test!(overridden: TimeTypes <- ["-tcr", "-tmod"]; Last => Ok(TimeTypes { accessed: false, modified: true, created: false })); diff --git a/xtests/error_value b/xtests/error_value index 287ed22..bda48bc 100644 --- a/xtests/error_value +++ b/xtests/error_value @@ -1 +1 @@ -Flag --time needs a value +Flag --time needs a value (choices: modified, accessed, created)