diff --git a/src/options/dir_action.rs b/src/options/dir_action.rs index d7884f5..d2cacaf 100644 --- a/src/options/dir_action.rs +++ b/src/options/dir_action.rs @@ -8,12 +8,12 @@ impl DirAction { /// Determine which action to perform when trying to list a directory. pub fn deduce(matches: &MatchedFlags) -> Result { - let recurse = matches.has(&flags::RECURSE); - let list = matches.has(&flags::LIST_DIRS); - let tree = matches.has(&flags::TREE); + let recurse = matches.has(&flags::RECURSE)?; + let list = matches.has(&flags::LIST_DIRS)?; + let tree = matches.has(&flags::TREE)?; // Early check for --level when it wouldn’t do anything - if !recurse && !tree && matches.get(&flags::LEVEL).is_some() { + if !recurse && !tree && matches.get(&flags::LEVEL)?.is_some() { return Err(Misfire::Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE)); } @@ -37,7 +37,7 @@ impl RecurseOptions { /// Determine which files should be recursed into. pub fn deduce(matches: &MatchedFlags, tree: bool) -> Result { - let max_depth = if let Some(level) = matches.get(&flags::LEVEL) { + let max_depth = if let Some(level) = matches.get(&flags::LEVEL)? { match level.to_string_lossy().parse() { Ok(l) => Some(l), Err(e) => return Err(Misfire::FailedParse(e)), @@ -56,9 +56,10 @@ impl RecurseOptions { mod test { use super::*; use options::flags; + use options::parser::Flag; macro_rules! test { - ($name:ident: $type:ident <- $inputs:expr => $result:expr) => { + ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => { #[test] fn $name() { use options::parser::Arg; @@ -66,32 +67,36 @@ mod test { use options::test::Strictnesses::*; static TEST_ARGS: &[&Arg] = &[&flags::RECURSE, &flags::LIST_DIRS, &flags::TREE, &flags::LEVEL ]; - assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result) + assert_parses($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf), $result) } }; } // Default behaviour - test!(empty: DirAction <- [] => Ok(DirAction::List)); + test!(empty: DirAction <- []; Both => Ok(DirAction::List)); // Listing files as directories - test!(dirs_short: DirAction <- ["-d"] => Ok(DirAction::AsFile)); - test!(dirs_long: DirAction <- ["--list-dirs"] => Ok(DirAction::AsFile)); + test!(dirs_short: DirAction <- ["-d"]; Both => Ok(DirAction::AsFile)); + test!(dirs_long: DirAction <- ["--list-dirs"]; Both => Ok(DirAction::AsFile)); // Recursing use self::DirAction::Recurse; - test!(rec_short: DirAction <- ["-R"] => Ok(Recurse(RecurseOptions { tree: false, max_depth: None }))); - test!(rec_long: DirAction <- ["--recurse"] => Ok(Recurse(RecurseOptions { tree: false, max_depth: None }))); - test!(rec_lim_short: DirAction <- ["-RL4"] => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(4) }))); - test!(rec_lim_short_2: DirAction <- ["-RL=5"] => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(5) }))); - test!(rec_lim_long: DirAction <- ["--recurse", "--level", "666"] => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(666) }))); - test!(rec_lim_long_2: DirAction <- ["--recurse", "--level=0118"] => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(118) }))); - test!(rec_tree: DirAction <- ["--recurse", "--tree"] => Ok(Recurse(RecurseOptions { tree: true, max_depth: None }))); - test!(rec_short_tree: DirAction <- ["--tree", "--recurse"] => Ok(Recurse(RecurseOptions { tree: true, max_depth: None }))); + test!(rec_short: DirAction <- ["-R"]; Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: None }))); + test!(rec_long: DirAction <- ["--recurse"]; Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: None }))); + test!(rec_lim_short: DirAction <- ["-RL4"]; Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(4) }))); + test!(rec_lim_short_2: DirAction <- ["-RL=5"]; Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(5) }))); + test!(rec_lim_long: DirAction <- ["--recurse", "--level", "666"]; Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(666) }))); + test!(rec_lim_long_2: DirAction <- ["--recurse", "--level=0118"]; Both => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(118) }))); + test!(rec_tree: DirAction <- ["--recurse", "--tree"]; Both => Ok(Recurse(RecurseOptions { tree: true, max_depth: None }))); + test!(rec_short_tree: DirAction <- ["--tree", "--recurse"]; Both => Ok(Recurse(RecurseOptions { tree: true, max_depth: None }))); // Errors - test!(error: DirAction <- ["--list-dirs", "--recurse"] => Err(Misfire::Conflict(&flags::RECURSE, &flags::LIST_DIRS))); - test!(error_2: DirAction <- ["--list-dirs", "--tree"] => Err(Misfire::Conflict(&flags::TREE, &flags::LIST_DIRS))); - test!(underwaterlevel: DirAction <- ["--level=4"] => Err(Misfire::Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE))); + test!(error: DirAction <- ["--list-dirs", "--recurse"]; Both => Err(Misfire::Conflict(&flags::RECURSE, &flags::LIST_DIRS))); + test!(error_2: DirAction <- ["--list-dirs", "--tree"]; Both => Err(Misfire::Conflict(&flags::TREE, &flags::LIST_DIRS))); + test!(underwaterlevel: DirAction <- ["--level=4"]; Both => Err(Misfire::Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE))); + + // Overriding + test!(overriding_1: DirAction <- ["-RL=6", "-L=7"]; Last => Ok(Recurse(RecurseOptions { tree: false, max_depth: Some(7) }))); + test!(overriding_2: DirAction <- ["-RL=6", "-L=7"]; Complain => Err(Misfire::Duplicate(Flag::Short(b'L'), Flag::Short(b'L')))); } diff --git a/src/options/filter.rs b/src/options/filter.rs index 0832db0..184b7d8 100644 --- a/src/options/filter.rs +++ b/src/options/filter.rs @@ -11,8 +11,8 @@ impl FileFilter { /// command-line arguments. pub fn deduce(matches: &MatchedFlags) -> Result { Ok(FileFilter { - list_dirs_first: matches.has(&flags::DIRS_FIRST), - reverse: matches.has(&flags::REVERSE), + list_dirs_first: matches.has(&flags::DIRS_FIRST)?, + reverse: matches.has(&flags::REVERSE)?, sort_field: SortField::deduce(matches)?, dot_filter: DotFilter::deduce(matches)?, ignore_patterns: IgnorePatterns::deduce(matches)?, @@ -38,7 +38,7 @@ impl SortField { /// argument. This will return `Err` if the option is there, but does not /// correspond to a valid field. fn deduce(matches: &MatchedFlags) -> Result { - let word = match matches.get(&flags::SORT) { + let word = match matches.get(&flags::SORT)? { Some(w) => w, None => return Ok(SortField::default()), }; @@ -85,11 +85,22 @@ impl SortField { impl DotFilter { pub fn deduce(matches: &MatchedFlags) -> Result { - match matches.count(&flags::ALL) { - 0 => Ok(DotFilter::JustFiles), - 1 => Ok(DotFilter::Dotfiles), - _ => if matches.has(&flags::TREE) { Err(Misfire::TreeAllAll) } - else { Ok(DotFilter::DotfilesAndDots) } + let count = matches.count(&flags::ALL); + + if count == 0 { + Ok(DotFilter::JustFiles) + } + else if count == 1 { + Ok(DotFilter::Dotfiles) + } + else if matches.count(&flags::TREE) > 0 { + Err(Misfire::TreeAllAll) + } + else if count >= 3 && matches.is_strict() { + Err(Misfire::Conflict(&flags::ALL, &flags::ALL)) + } + else { + Ok(DotFilter::DotfilesAndDots) } } } @@ -101,7 +112,7 @@ impl IgnorePatterns { /// command-line arguments. pub fn deduce(matches: &MatchedFlags) -> Result { - let inputs = match matches.get(&flags::IGNORE_GLOB) { + let inputs = match matches.get(&flags::IGNORE_GLOB)? { None => return Ok(IgnorePatterns::empty()), Some(is) => is, }; @@ -126,6 +137,7 @@ mod test { use super::*; use std::ffi::OsString; use options::flags; + use options::parser::Flag; pub fn os(input: &'static str) -> OsString { let mut os = OsString::new(); @@ -134,7 +146,7 @@ mod test { } macro_rules! test { - ($name:ident: $type:ident <- $inputs:expr => $result:expr) => { + ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => { #[test] fn $name() { use options::parser::Arg; @@ -142,7 +154,7 @@ mod test { use options::test::Strictnesses::*; static TEST_ARGS: &[&Arg] = &[ &flags::SORT, &flags::ALL, &flags::TREE, &flags::IGNORE_GLOB ]; - assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result) + assert_parses($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf), $result) } }; } @@ -151,21 +163,23 @@ mod test { use super::*; // Default behaviour - test!(empty: SortField <- [] => Ok(SortField::default())); + test!(empty: SortField <- []; Both => Ok(SortField::default())); // Sort field arguments - test!(one_arg: SortField <- ["--sort=cr"] => Ok(SortField::CreatedDate)); - test!(one_long: SortField <- ["--sort=size"] => Ok(SortField::Size)); - test!(one_short: SortField <- ["-saccessed"] => Ok(SortField::AccessedDate)); - test!(lowercase: SortField <- ["--sort", "name"] => Ok(SortField::Name(SortCase::Sensitive))); - test!(uppercase: SortField <- ["--sort", "Name"] => Ok(SortField::Name(SortCase::Insensitive))); + test!(one_arg: SortField <- ["--sort=cr"]; Both => Ok(SortField::CreatedDate)); + test!(one_long: SortField <- ["--sort=size"]; Both => Ok(SortField::Size)); + test!(one_short: SortField <- ["-saccessed"]; Both => Ok(SortField::AccessedDate)); + test!(lowercase: SortField <- ["--sort", "name"]; Both => Ok(SortField::Name(SortCase::Sensitive))); + test!(uppercase: SortField <- ["--sort", "Name"]; Both => Ok(SortField::Name(SortCase::Insensitive))); // Errors - test!(error: SortField <- ["--sort=colour"] => Err(Misfire::bad_argument(&flags::SORT, &os("colour"), super::SORTS))); + test!(error: SortField <- ["--sort=colour"]; Both => Err(Misfire::bad_argument(&flags::SORT, &os("colour"), super::SORTS))); // Overriding - test!(overridden: SortField <- ["--sort=cr", "--sort", "mod"] => Ok(SortField::ModifiedDate)); - test!(overridden_2: SortField <- ["--sort", "none", "--sort=Extension"] => Ok(SortField::Extension(SortCase::Insensitive))); + test!(overridden: SortField <- ["--sort=cr", "--sort", "mod"]; Last => Ok(SortField::ModifiedDate)); + test!(overridden_2: SortField <- ["--sort", "none", "--sort=Extension"]; Last => Ok(SortField::Extension(SortCase::Insensitive))); + test!(overridden_3: SortField <- ["--sort=cr", "--sort", "mod"]; Complain => Err(Misfire::Duplicate(Flag::Long("sort"), Flag::Long("sort")))); + test!(overridden_4: SortField <- ["--sort", "none", "--sort=Extension"]; Complain => Err(Misfire::Duplicate(Flag::Long("sort"), Flag::Long("sort")))); } @@ -173,16 +187,20 @@ mod test { use super::*; // Default behaviour - test!(empty: DotFilter <- [] => Ok(DotFilter::JustFiles)); + test!(empty: DotFilter <- []; Both => Ok(DotFilter::JustFiles)); // --all - test!(all: DotFilter <- ["--all"] => Ok(DotFilter::Dotfiles)); - test!(all_all: DotFilter <- ["--all", "-a"] => Ok(DotFilter::DotfilesAndDots)); - test!(all_all_2: DotFilter <- ["-aa"] => Ok(DotFilter::DotfilesAndDots)); + test!(all: DotFilter <- ["--all"]; Both => Ok(DotFilter::Dotfiles)); + test!(all_all: DotFilter <- ["--all", "-a"]; Both => Ok(DotFilter::DotfilesAndDots)); + test!(all_all_2: DotFilter <- ["-aa"]; Both => Ok(DotFilter::DotfilesAndDots)); + + test!(all_all_3: DotFilter <- ["-aaa"]; Last => Ok(DotFilter::DotfilesAndDots)); + test!(all_all_4: DotFilter <- ["-aaa"]; Complain => Err(Misfire::Conflict(&flags::ALL, &flags::ALL))); // --all and --tree - test!(tree_a: DotFilter <- ["-Ta"] => Ok(DotFilter::Dotfiles)); - test!(tree_aa: DotFilter <- ["-Taa"] => Err(Misfire::TreeAllAll)); + test!(tree_a: DotFilter <- ["-Ta"]; Both => Ok(DotFilter::Dotfiles)); + test!(tree_aa: DotFilter <- ["-Taa"]; Both => Err(Misfire::TreeAllAll)); + test!(tree_aaa: DotFilter <- ["-Taaa"]; Both => Err(Misfire::TreeAllAll)); } @@ -196,9 +214,15 @@ mod test { } // Various numbers of globs - test!(none: IgnorePatterns <- [] => Ok(IgnorePatterns::empty())); - test!(one: IgnorePatterns <- ["--ignore-glob", "*.ogg"] => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg") ]))); - test!(two: IgnorePatterns <- ["--ignore-glob=*.ogg|*.MP3"] => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg"), pat("*.MP3") ]))); - test!(loads: IgnorePatterns <- ["-I*|?|.|*"] => Ok(IgnorePatterns::from_iter(vec![ pat("*"), pat("?"), pat("."), pat("*") ]))); + test!(none: IgnorePatterns <- []; Both => Ok(IgnorePatterns::empty())); + test!(one: IgnorePatterns <- ["--ignore-glob", "*.ogg"]; Both => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg") ]))); + test!(two: IgnorePatterns <- ["--ignore-glob=*.ogg|*.MP3"]; Both => Ok(IgnorePatterns::from_iter(vec![ pat("*.ogg"), pat("*.MP3") ]))); + test!(loads: IgnorePatterns <- ["-I*|?|.|*"]; Both => Ok(IgnorePatterns::from_iter(vec![ pat("*"), pat("?"), pat("."), pat("*") ]))); + + // Overriding + test!(overridden: IgnorePatterns <- ["-I=*.ogg", "-I", "*.mp3"]; Last => Ok(IgnorePatterns::from_iter(vec![ pat("*.mp3") ]))); + test!(overridden_2: IgnorePatterns <- ["-I", "*.OGG", "-I*.MP3"]; Last => Ok(IgnorePatterns::from_iter(vec![ pat("*.MP3") ]))); + test!(overridden_3: IgnorePatterns <- ["-I=*.ogg", "-I", "*.mp3"]; Complain => Err(Misfire::Duplicate(Flag::Short(b'I'), Flag::Short(b'I')))); + test!(overridden_4: IgnorePatterns <- ["-I", "*.OGG", "-I*.MP3"]; Complain => Err(Misfire::Duplicate(Flag::Short(b'I'), Flag::Short(b'I')))); } } diff --git a/src/options/help.rs b/src/options/help.rs index ce7e15a..8df31c0 100644 --- a/src/options/help.rs +++ b/src/options/help.rs @@ -72,9 +72,13 @@ impl HelpString { /// Determines how to show help, if at all, based on the user’s /// command-line arguments. This one works backwards from the other /// ‘deduce’ functions, returning Err if help needs to be shown. + /// + /// We don’t do any strict-mode error checking here: it’s OK to give + /// the --help or --long flags more than once. Actually checking for + /// errors when the user wants help is kind of petty! pub fn deduce(matches: &MatchedFlags) -> Result<(), HelpString> { - if matches.has(&flags::HELP) { - let only_long = matches.has(&flags::LONG); + if matches.count(&flags::HELP) > 0 { + let only_long = matches.count(&flags::LONG) > 0; let git = cfg!(feature="git"); let xattrs = xattr::ENABLED; Err(HelpString { only_long, git, xattrs }) diff --git a/src/options/misfire.rs b/src/options/misfire.rs index d89f269..d7e0b50 100644 --- a/src/options/misfire.rs +++ b/src/options/misfire.rs @@ -5,7 +5,7 @@ use std::num::ParseIntError; use glob; use options::{HelpString, VersionString}; -use options::parser::{Arg, ParseError}; +use options::parser::{Arg, Flag, ParseError}; /// A list of legal choices for an argument-taking option @@ -36,6 +36,9 @@ pub enum Misfire { /// The user wanted the version number. Version(VersionString), + /// An option was given twice or more in strict mode. + Duplicate(Flag, Flag), + /// Two options were given that conflict with one another. Conflict(&'static Arg, &'static Arg), @@ -93,6 +96,7 @@ impl fmt::Display for Misfire { Help(ref text) => write!(f, "{}", text), Version(ref version) => write!(f, "{}", version), Conflict(ref a, ref b) => write!(f, "Option {} conflicts with option {}.", a, b), + Duplicate(ref a, ref b) => write!(f, "Flag {:?} conflicts with flag {:?}.", a, b), Useless(ref a, false, ref b) => write!(f, "Option {} is useless without option {}.", a, b), Useless(ref a, true, ref b) => write!(f, "Option {} is useless given option {}.", a, b), Useless2(ref a, ref b1, ref b2) => write!(f, "Option {} is useless without options {} or {}.", a, b1, b2), diff --git a/src/options/mod.rs b/src/options/mod.rs index 8a9d805..0299cad 100644 --- a/src/options/mod.rs +++ b/src/options/mod.rs @@ -170,6 +170,9 @@ pub mod test { Both, } + /// This function gets used by the other testing modules. + /// It can run with one or both strictness values: if told to run with + /// both, then both should resolve to the same result. pub fn assert_parses(inputs: &[&str], args: &'static [&'static Arg], strictnesses: Strictnesses, get: F, result: T) where T: PartialEq + Debug, F: Fn(&MatchedFlags) -> T { diff --git a/src/options/parser.rs b/src/options/parser.rs index 55ed97c..882808f 100644 --- a/src/options/parser.rs +++ b/src/options/parser.rs @@ -31,6 +31,8 @@ use std::ffi::{OsStr, OsString}; use std::fmt; +use options::Misfire; + /// A **short argument** is a single ASCII character. pub type ShortArg = u8; @@ -50,7 +52,7 @@ pub enum Flag { } impl Flag { - fn matches(&self, arg: &Arg) -> bool { + pub fn matches(&self, arg: &Arg) -> bool { match *self { Flag::Short(short) => arg.short == Some(short), Flag::Long(long) => arg.long == long, @@ -312,21 +314,55 @@ pub struct MatchedFlags<'args> { strictness: Strictness, } +use self::Strictness::*; impl<'a> MatchedFlags<'a> { /// Whether the given argument was specified. - pub fn has(&self, arg: &Arg) -> bool { - self.flags.iter().rev() - .find(|tuple| tuple.1.is_none() && tuple.0.matches(arg)) - .is_some() + pub fn has(&self, arg: &'static Arg) -> Result { + match self.strictness { + UseLastArguments => { + let any = self.flags.iter().rev() + .find(|tuple| tuple.1.is_none() && tuple.0.matches(arg)) + .is_some(); + Ok(any) + } + ComplainAboutRedundantArguments => { + let all = self.flags.iter() + .filter(|tuple| tuple.1.is_none() && tuple.0.matches(arg)) + .collect::>(); + + if all.len() < 2 { Ok(all.len() == 1) } + else { Err(Misfire::Conflict(arg, arg)) } + } + } + } + + // This code could probably be better. + + pub fn get(&self, arg: &'static Arg) -> Result, Misfire> { + self.get_where(|flag| flag.matches(arg)) } /// If the given argument was specified, return its value. /// The value is not guaranteed to be valid UTF-8. - pub fn get(&self, arg: &Arg) -> Option<&OsStr> { - self.flags.iter().rev() - .find(|tuple| tuple.1.is_some() && tuple.0.matches(arg)) - .map(|tuple| tuple.1.unwrap()) + pub fn get_where

(&self, predicate: P) -> Result, Misfire> + where P: Fn(&Flag) -> bool { + match self.strictness { + UseLastArguments => { + let found = self.flags.iter().rev() + .find(|tuple| tuple.1.is_some() && predicate(&tuple.0)) + .map(|tuple| tuple.1.unwrap()); + Ok(found) + } + ComplainAboutRedundantArguments => { + let those = self.flags.iter() + .filter(|tuple| tuple.1.is_some() && predicate(&tuple.0)) + .collect::>(); + + if those.len() < 2 { Ok(those.first().cloned().map(|t| t.1.unwrap())) } + else { Err(Misfire::Duplicate(those[0].0.clone(), those[1].0.clone())) } + } + } } // It’s annoying that ‘has’ and ‘get’ won’t work when accidentally given @@ -338,6 +374,12 @@ impl<'a> MatchedFlags<'a> { .filter(|tuple| tuple.0.matches(arg)) .count() } + + /// Checks whether strict mode is on. This is usually done from within + /// ‘has’ and ‘get’, but it’s available in an emergency. + pub fn is_strict(&self) -> bool { + self.strictness == Strictness::ComplainAboutRedundantArguments + } } @@ -547,7 +589,7 @@ mod matches_test { strictness: Strictness::UseLastArguments, }; - assert_eq!(flags.has(&$param), $result); + assert_eq!(flags.has(&$param), Ok($result)); } }; } @@ -573,7 +615,7 @@ mod matches_test { strictness: Strictness::UseLastArguments, }; - assert_eq!(flags.get(&COUNT), Some(&*everything)); + assert_eq!(flags.get(&COUNT), Ok(Some(&*everything))); } #[test] @@ -587,13 +629,13 @@ mod matches_test { strictness: Strictness::UseLastArguments, }; - assert_eq!(flags.get(&COUNT), Some(&*nothing)); + assert_eq!(flags.get(&COUNT), Ok(Some(&*nothing))); } #[test] fn no_count() { let flags = MatchedFlags { flags: Vec::new(), strictness: Strictness::UseLastArguments }; - assert!(!flags.has(&COUNT)); + assert!(!flags.has(&COUNT).unwrap()); } } diff --git a/src/options/version.rs b/src/options/version.rs index 79197f1..fb3759e 100644 --- a/src/options/version.rs +++ b/src/options/version.rs @@ -17,8 +17,10 @@ impl VersionString { /// Determines how to show the version, if at all, based on the user’s /// command-line arguments. This one works backwards from the other /// ‘deduce’ functions, returning Err if help needs to be shown. + /// + /// Like --help, this doesn’t bother checking for errors. pub fn deduce(matches: &MatchedFlags) -> Result<(), VersionString> { - if matches.has(&flags::VERSION) { + if matches.count(&flags::VERSION) > 0 { Err(VersionString { cargo: env!("CARGO_PKG_VERSION") }) } else { diff --git a/src/options/view.rs b/src/options/view.rs index 58202a8..5fce5ab 100644 --- a/src/options/view.rs +++ b/src/options/view.rs @@ -19,7 +19,7 @@ impl View { pub fn deduce(matches: &MatchedFlags) -> Result { let mode = Mode::deduce(matches)?; let colours = Colours::deduce(matches)?; - let style = FileStyle::deduce(matches); + let style = FileStyle::deduce(matches)?; Ok(View { mode, colours, style }) } } @@ -32,17 +32,17 @@ impl Mode { use options::misfire::Misfire::*; let long = || { - if matches.has(&flags::ACROSS) && !matches.has(&flags::GRID) { + if matches.has(&flags::ACROSS)? && !matches.has(&flags::GRID)? { Err(Useless(&flags::ACROSS, true, &flags::LONG)) } - else if matches.has(&flags::ONE_LINE) { + else if matches.has(&flags::ONE_LINE)? { Err(Useless(&flags::ONE_LINE, true, &flags::LONG)) } else { Ok(details::Options { table: Some(TableOptions::deduce(matches)?), - header: matches.has(&flags::HEADER), - xattr: xattr::ENABLED && matches.has(&flags::EXTENDED), + header: matches.has(&flags::HEADER)?, + xattr: xattr::ENABLED && matches.has(&flags::EXTENDED)?, }) } }; @@ -50,15 +50,15 @@ impl Mode { let long_options_scan = || { for option in &[ &flags::BINARY, &flags::BYTES, &flags::INODE, &flags::LINKS, &flags::HEADER, &flags::BLOCKS, &flags::TIME, &flags::GROUP ] { - if matches.has(option) { + if matches.has(option)? { return Err(Useless(*option, false, &flags::LONG)); } } - if cfg!(feature="git") && matches.has(&flags::GIT) { + if cfg!(feature="git") && matches.has(&flags::GIT)? { Err(Useless(&flags::GIT, false, &flags::LONG)) } - else if matches.has(&flags::LEVEL) && !matches.has(&flags::RECURSE) && !matches.has(&flags::TREE) { + else if matches.has(&flags::LEVEL)? && !matches.has(&flags::RECURSE)? && !matches.has(&flags::TREE)? { Err(Useless2(&flags::LEVEL, &flags::RECURSE, &flags::TREE)) } else { @@ -68,26 +68,26 @@ impl Mode { let other_options_scan = || { if let Some(width) = TerminalWidth::deduce()?.width() { - if matches.has(&flags::ONE_LINE) { - if matches.has(&flags::ACROSS) { + if matches.has(&flags::ONE_LINE)? { + if matches.has(&flags::ACROSS)? { Err(Useless(&flags::ACROSS, true, &flags::ONE_LINE)) } else { Ok(Mode::Lines) } } - else if matches.has(&flags::TREE) { + else if matches.has(&flags::TREE)? { let details = details::Options { table: None, header: false, - xattr: xattr::ENABLED && matches.has(&flags::EXTENDED), + xattr: xattr::ENABLED && matches.has(&flags::EXTENDED)?, }; Ok(Mode::Details(details)) } else { let grid = grid::Options { - across: matches.has(&flags::ACROSS), + across: matches.has(&flags::ACROSS)?, console_width: width, }; @@ -99,11 +99,11 @@ impl Mode { // as the program’s stdout being connected to a file, then // fallback to the lines view. - if matches.has(&flags::TREE) { + if matches.has(&flags::TREE)? { let details = details::Options { table: None, header: false, - xattr: xattr::ENABLED && matches.has(&flags::EXTENDED), + xattr: xattr::ENABLED && matches.has(&flags::EXTENDED)?, }; Ok(Mode::Details(details)) @@ -114,9 +114,9 @@ impl Mode { } }; - if matches.has(&flags::LONG) { + if matches.has(&flags::LONG)? { let details = long()?; - if matches.has(&flags::GRID) { + if matches.has(&flags::GRID)? { match other_options_scan()? { Mode::Grid(grid) => return Ok(Mode::GridDetails(grid, details)), others => return Ok(others), @@ -185,11 +185,11 @@ impl TableOptions { time_format: TimeFormat::deduce(matches)?, size_format: SizeFormat::deduce(matches)?, time_types: TimeTypes::deduce(matches)?, - inode: matches.has(&flags::INODE), - links: matches.has(&flags::LINKS), - blocks: matches.has(&flags::BLOCKS), - group: matches.has(&flags::GROUP), - git: cfg!(feature="git") && matches.has(&flags::GIT), + inode: matches.has(&flags::INODE)?, + links: matches.has(&flags::LINKS)?, + blocks: matches.has(&flags::BLOCKS)?, + group: matches.has(&flags::GROUP)?, + git: cfg!(feature="git") && matches.has(&flags::GIT)?, }) } } @@ -206,8 +206,8 @@ impl SizeFormat { /// involves the `--binary` or `--bytes` flags, and these conflict with /// each other. fn deduce(matches: &MatchedFlags) -> Result { - let binary = matches.has(&flags::BINARY); - let bytes = matches.has(&flags::BYTES); + let binary = matches.has(&flags::BINARY)?; + let bytes = matches.has(&flags::BYTES)?; match (binary, bytes) { (true, true ) => Err(Misfire::Conflict(&flags::BINARY, &flags::BYTES)), @@ -226,7 +226,7 @@ impl TimeFormat { pub use output::time::{DefaultFormat, ISOFormat}; const STYLES: &[&str] = &["default", "long-iso", "full-iso", "iso"]; - let word = match matches.get(&flags::TIME_STYLE) { + let word = match matches.get(&flags::TIME_STYLE)? { Some(w) => w, None => return Ok(TimeFormat::DefaultFormat(DefaultFormat::new())), }; @@ -265,10 +265,10 @@ impl TimeTypes { /// option, but passing *no* options means that the user just wants to /// see the default set. fn deduce(matches: &MatchedFlags) -> Result { - let possible_word = matches.get(&flags::TIME); - let modified = matches.has(&flags::MODIFIED); - let created = matches.has(&flags::CREATED); - let accessed = matches.has(&flags::ACCESSED); + let possible_word = matches.get(&flags::TIME)?; + let modified = matches.has(&flags::MODIFIED)?; + let created = matches.has(&flags::CREATED)?; + let accessed = matches.has(&flags::ACCESSED)?; if let Some(word) = possible_word { if modified { @@ -329,13 +329,14 @@ impl Default for TerminalColours { } } +const COLOURS: &[&str] = &["always", "auto", "never"]; + impl TerminalColours { /// Determine which terminal colour conditions to use. fn deduce(matches: &MatchedFlags) -> Result { - const COLOURS: &[&str] = &["always", "auto", "never"]; - let word = match matches.get(&flags::COLOR).or_else(|| matches.get(&flags::COLOUR)) { + let word = match matches.get_where(|f| f.matches(&flags::COLOR) || f.matches(&flags::COLOUR))? { Some(w) => w, None => return Ok(TerminalColours::default()), }; @@ -362,7 +363,7 @@ impl Colours { let tc = TerminalColours::deduce(matches)?; if tc == Always || (tc == Automatic && TERM_WIDTH.is_some()) { - let scale = matches.has(&flags::COLOR_SCALE) || matches.has(&flags::COLOUR_SCALE); + let scale = matches.has(&flags::COLOR_SCALE)? || matches.has(&flags::COLOUR_SCALE)?; Ok(Colours::colourful(scale)) } else { @@ -374,17 +375,19 @@ impl Colours { impl FileStyle { - fn deduce(matches: &MatchedFlags) -> FileStyle { - let classify = Classify::deduce(matches); + fn deduce(matches: &MatchedFlags) -> Result { + let classify = Classify::deduce(matches)?; let exts = FileExtensions; - FileStyle { classify, exts } + Ok(FileStyle { classify, exts }) } } impl Classify { - fn deduce(matches: &MatchedFlags) -> Classify { - if matches.has(&flags::CLASSIFY) { Classify::AddFileIndicators } - else { Classify::JustFilenames } + fn deduce(matches: &MatchedFlags) -> Result { + let flagged = matches.has(&flags::CLASSIFY)?; + + Ok(if flagged { Classify::AddFileIndicators } + else { Classify::JustFilenames }) } } @@ -407,10 +410,18 @@ lazy_static! { #[cfg(test)] mod test { use super::*; + use std::ffi::OsString; 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 => $result:expr) => { + ($name:ident: $type:ident <- $inputs:expr; $stricts:expr => $result:expr) => { #[test] fn $name() { use options::parser::Arg; @@ -418,9 +429,10 @@ mod test { use options::test::Strictnesses::*; static TEST_ARGS: &[&Arg] = &[ &flags::BINARY, &flags::BYTES, - &flags::TIME, &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED ]; + &flags::TIME, &flags::MODIFIED, &flags::CREATED, &flags::ACCESSED, + &flags::COLOR, &flags::COLOUR ]; - assert_parses($inputs.as_ref(), TEST_ARGS, Both, |mf| $type::deduce(mf), $result) + assert_parses($inputs.as_ref(), TEST_ARGS, $stricts, |mf| $type::deduce(mf), $result) } }; } @@ -429,49 +441,75 @@ mod test { mod size_formats { use super::*; - test!(empty: SizeFormat <- [] => Ok(SizeFormat::DecimalBytes)); - test!(binary: SizeFormat <- ["--binary"] => Ok(SizeFormat::BinaryBytes)); - test!(bytes: SizeFormat <- ["--bytes"] => Ok(SizeFormat::JustBytes)); - test!(both: SizeFormat <- ["--binary", "--bytes"] => Err(Misfire::Conflict(&flags::BINARY, &flags::BYTES))); + test!(empty: SizeFormat <- []; Both => Ok(SizeFormat::DecimalBytes)); + test!(binary: SizeFormat <- ["--binary"]; Both => Ok(SizeFormat::BinaryBytes)); + test!(bytes: SizeFormat <- ["--bytes"]; Both => Ok(SizeFormat::JustBytes)); + test!(both: SizeFormat <- ["--binary", "--bytes"]; Both => Err(Misfire::Conflict(&flags::BINARY, &flags::BYTES))); } mod time_types { use super::*; - use std::ffi::OsString; - - fn os(input: &'static str) -> OsString { - let mut os = OsString::new(); - os.push(input); - os - } - // Default behaviour - test!(empty: TimeTypes <- [] => Ok(TimeTypes::default())); - test!(modified: TimeTypes <- ["--modified"] => Ok(TimeTypes { accessed: false, modified: true, created: false })); - test!(m: TimeTypes <- ["-m"] => Ok(TimeTypes { accessed: false, modified: true, created: false })); - test!(time_mod: TimeTypes <- ["--time=modified"] => Ok(TimeTypes { accessed: false, modified: true, created: false })); - test!(time_m: TimeTypes <- ["-tmod"] => Ok(TimeTypes { accessed: false, modified: true, created: false })); + test!(empty: TimeTypes <- []; Both => Ok(TimeTypes::default())); + test!(modified: TimeTypes <- ["--modified"]; Both => Ok(TimeTypes { accessed: false, modified: true, created: false })); + test!(m: TimeTypes <- ["-m"]; Both => Ok(TimeTypes { accessed: false, modified: true, created: false })); + test!(time_mod: TimeTypes <- ["--time=modified"]; Both => Ok(TimeTypes { accessed: false, modified: true, created: false })); + test!(time_m: TimeTypes <- ["-tmod"]; Both => Ok(TimeTypes { accessed: false, modified: true, created: false })); - test!(acc: TimeTypes <- ["--accessed"] => Ok(TimeTypes { accessed: true, modified: false, created: false })); - test!(a: TimeTypes <- ["-u"] => Ok(TimeTypes { accessed: true, modified: false, created: false })); - test!(time_acc: TimeTypes <- ["--time", "accessed"] => Ok(TimeTypes { accessed: true, modified: false, created: false })); - test!(time_a: TimeTypes <- ["-t", "acc"] => Ok(TimeTypes { accessed: true, modified: false, created: false })); + test!(acc: TimeTypes <- ["--accessed"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: false })); + test!(a: TimeTypes <- ["-u"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: false })); + test!(time_acc: TimeTypes <- ["--time", "accessed"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: false })); + test!(time_a: TimeTypes <- ["-t", "acc"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: false })); - test!(cr: TimeTypes <- ["--created"] => Ok(TimeTypes { accessed: false, modified: false, created: true })); - test!(c: TimeTypes <- ["-U"] => Ok(TimeTypes { accessed: false, modified: false, created: true })); - test!(time_cr: TimeTypes <- ["--time=created"] => Ok(TimeTypes { accessed: false, modified: false, created: true })); - test!(time_c: TimeTypes <- ["-tcr"] => Ok(TimeTypes { accessed: false, modified: false, created: true })); + test!(cr: TimeTypes <- ["--created"]; Both => Ok(TimeTypes { accessed: false, modified: false, created: true })); + test!(c: TimeTypes <- ["-U"]; Both => Ok(TimeTypes { accessed: false, modified: false, created: true })); + test!(time_cr: TimeTypes <- ["--time=created"]; Both => Ok(TimeTypes { accessed: false, modified: false, created: true })); + test!(time_c: TimeTypes <- ["-tcr"]; Both => Ok(TimeTypes { accessed: false, modified: false, created: true })); // Multiples - test!(time_uu: TimeTypes <- ["-uU"] => Ok(TimeTypes { accessed: true, modified: false, created: true })); - - // Overriding - test!(time_mc: TimeTypes <- ["-tcr", "-tmod"] => Ok(TimeTypes { accessed: false, modified: true, created: false })); + test!(time_uu: TimeTypes <- ["-uU"]; Both => Ok(TimeTypes { accessed: true, modified: false, created: true })); // Errors - test!(time_tea: TimeTypes <- ["--time=tea"] => Err(Misfire::bad_argument(&flags::TIME, &os("tea"), super::TIMES))); - test!(time_ea: TimeTypes <- ["-tea"] => Err(Misfire::bad_argument(&flags::TIME, &os("ea"), super::TIMES))); + 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))); + + // Overriding + test!(overridden: TimeTypes <- ["-tcr", "-tmod"]; Last => Ok(TimeTypes { accessed: false, modified: true, created: false })); + test!(overridden_2: TimeTypes <- ["-tcr", "-tmod"]; Complain => Err(Misfire::Duplicate(Flag::Short(b't'), Flag::Short(b't')))); + } + + + mod colourses { + use super::*; + + // Default + test!(empty: TerminalColours <- []; Both => Ok(TerminalColours::default())); + + // --colour + test!(u_always: TerminalColours <- ["--colour=always"]; Both => Ok(TerminalColours::Always)); + test!(u_auto: TerminalColours <- ["--colour", "auto"]; Both => Ok(TerminalColours::Automatic)); + test!(u_never: TerminalColours <- ["--colour=never"]; Both => Ok(TerminalColours::Never)); + + // --color + test!(no_u_always: TerminalColours <- ["--color", "always"]; Both => Ok(TerminalColours::Always)); + test!(no_u_auto: TerminalColours <- ["--color=auto"]; Both => Ok(TerminalColours::Automatic)); + test!(no_u_never: TerminalColours <- ["--color", "never"]; Both => Ok(TerminalColours::Never)); + + // Errors + test!(no_u_error: TerminalColours <- ["--color=upstream"]; Both => Err(Misfire::bad_argument(&flags::COLOR, &os("upstream"), super::COLOURS))); // the error is for --color + test!(u_error: TerminalColours <- ["--colour=lovers"]; Both => Err(Misfire::bad_argument(&flags::COLOR, &os("lovers"), super::COLOURS))); // and so is this one! + + // Overriding + test!(overridden_1: TerminalColours <- ["--colour=auto", "--colour=never"]; Last => Ok(TerminalColours::Never)); + test!(overridden_2: TerminalColours <- ["--color=auto", "--colour=never"]; Last => Ok(TerminalColours::Never)); + test!(overridden_3: TerminalColours <- ["--colour=auto", "--color=never"]; Last => Ok(TerminalColours::Never)); + test!(overridden_4: TerminalColours <- ["--color=auto", "--color=never"]; Last => Ok(TerminalColours::Never)); + + test!(overridden_5: TerminalColours <- ["--colour=auto", "--colour=never"]; Complain => Err(Misfire::Duplicate(Flag::Long("colour"), Flag::Long("colour")))); + test!(overridden_6: TerminalColours <- ["--color=auto", "--colour=never"]; Complain => Err(Misfire::Duplicate(Flag::Long("color"), Flag::Long("colour")))); + test!(overridden_7: TerminalColours <- ["--colour=auto", "--color=never"]; Complain => Err(Misfire::Duplicate(Flag::Long("colour"), Flag::Long("color")))); + test!(overridden_8: TerminalColours <- ["--color=auto", "--color=never"]; Complain => Err(Misfire::Duplicate(Flag::Long("color"), Flag::Long("color")))); } }