diff options
author | Benjamin Sago <ogham@bsago.me> | 2017-09-14 11:12:34 +0100 |
---|---|---|
committer | Benjamin Sago <ogham@bsago.me> | 2017-09-14 11:12:34 +0100 |
commit | f55bd6de5351007325df504f9efdf1d7183059d5 (patch) | |
tree | 5ab64a4f596bfa9aad005aebc778d7371a4c9b60 | |
parent | 19b77807551fa44d329c40a18c18438412c0b38a (diff) | |
parent | c475cccce468a7177674c7dbb189192af64deb37 (diff) |
Merge branch 'it’s-that-time-again'
-rw-r--r-- | README.md | 2 | ||||
-rw-r--r-- | contrib/completions.bash | 2 | ||||
-rw-r--r-- | contrib/completions.fish | 5 | ||||
-rw-r--r-- | contrib/completions.zsh | 2 | ||||
-rw-r--r-- | contrib/man/exa.1 | 1 | ||||
-rw-r--r-- | src/bin/main.rs | 8 | ||||
-rw-r--r-- | src/fs/filter.rs | 18 | ||||
-rw-r--r-- | src/options/filter.rs | 31 | ||||
-rw-r--r-- | src/options/flags.rs | 22 | ||||
-rw-r--r-- | src/options/help.rs | 3 | ||||
-rw-r--r-- | src/options/misfire.rs | 75 | ||||
-rw-r--r-- | src/options/parser.rs | 69 | ||||
-rw-r--r-- | src/options/style.rs | 13 | ||||
-rw-r--r-- | src/options/view.rs | 21 | ||||
-rw-r--r-- | xtests/dates_deifidom | 4 | ||||
-rw-r--r-- | xtests/error_lt | 2 | ||||
-rw-r--r-- | xtests/error_ltr | 2 | ||||
-rw-r--r-- | xtests/error_value | 2 | ||||
-rw-r--r-- | xtests/help | 3 | ||||
-rwxr-xr-x | xtests/run.sh | 9 |
20 files changed, 187 insertions, 107 deletions
@@ -54,7 +54,7 @@ These options are available when running with --long (`-l`): - **--time-style**: how to format timestamps - Valid **--color** options are **always**, **automatic**, and **never**. -- Valid sort fields are **accessed**, **created**, **extension**, **Extension**, **inode**, **modified**, **name**, **Name**, **size**, **type**, and **none**. Fields starting with a capital letter sort uppercase before lowercase. +- Valid sort fields are **accessed**, **created**, **extension**, **Extension**, **inode**, **modified**, **name**, **Name**, **size**, **type**, and **none**. Fields starting with a capital letter sort uppercase before lowercase. The modified field has the aliases **date**, **time**, and **newest**, while its reverse has the aliases **age** and **oldest**. - Valid time fields are **modified**, **accessed**, and **created**. - Valid time styles are **default**, **iso**, **long-iso**, and **full-iso**. diff --git a/contrib/completions.bash b/contrib/completions.bash index 9a54c23..c058ba4 100644 --- a/contrib/completions.bash +++ b/contrib/completions.bash @@ -14,7 +14,7 @@ _exa() ;; -s|--sort) - COMPREPLY=( $( compgen -W 'name filename Name Filename size filesize extension Extension modified accessed created type inode none --' -- "$cur" ) ) + COMPREPLY=( $( compgen -W 'name filename Name Filename size filesize extension Extension date time modified accessed created type inode oldest newest age none --' -- "$cur" ) ) return ;; diff --git a/contrib/completions.fish b/contrib/completions.fish index 57462b0..8cf43b0 100644 --- a/contrib/completions.fish +++ b/contrib/completions.fish @@ -23,7 +23,9 @@ complete -c exa -s 'L' -l 'level' -d "Limit the depth of recursion" -a "1 2 complete -c exa -s 'r' -l 'reverse' -d "Reverse the sort order" complete -c exa -s 's' -l 'sort' -x -d "Which field to sort by" -a " accessed\t'Sort by file accessed time' + age\t'Sort by file modified time (newest first)' created\t'Sort by file modified time' + date\t'Sort by file modified time' ext\t'Sort by file extension' Ext\t'Sort by file extension (uppercase first)' extension\t'Sort by file extension' @@ -34,8 +36,11 @@ complete -c exa -s 's' -l 'sort' -x -d "Which field to sort by" -a " modified\t'Sort by file modified time' name\t'Sort by filename' Name\t'Sort by filename (uppercase first)' + newest\t'Sort by file modified time (newest first)' none\t'Do not sort files at all' + oldest\t'Sort by file modified time' size\t'Sort by file size' + time\t'Sort by file modified time' type\t'Sort by file type' " diff --git a/contrib/completions.zsh b/contrib/completions.zsh index 0f5f04d..6536e2c 100644 --- a/contrib/completions.zsh +++ b/contrib/completions.zsh @@ -18,7 +18,7 @@ __exa() { {-d,--list-dirs}"[List directories like regular files]" \ {-L,--level}"+[Limit the depth of recursion]" \ {-r,--reverse}"[Reverse the sort order]" \ - {-s,--sort}"[Which field to sort by]:(sort field):(accessed created extension Extension filename Filename inode modified name Name none size type)" \ + {-s,--sort}"[Which field to sort by]:(sort field):(accessed age created date extension Extension filename Filename inode modified oldest name Name newest none size time type)" \ {-I,--ignore-glob}"[Ignore files that match these glob patterns]" \ {-b,--binary}"[List file sizes with binary prefixes]" \ {-B,--bytes}"[List file sizes in bytes, without any prefixes]" \ diff --git a/contrib/man/exa.1 b/contrib/man/exa.1 index 86d0703..dadb6c4 100644 --- a/contrib/man/exa.1 +++ b/contrib/man/exa.1 @@ -77,6 +77,7 @@ reverse the sort order .B \-s, \-\-sort=\f[I]SORT_FIELD\f[] which field to sort by. Valid fields are name, Name, extension, Extension, size, modified, accessed, created, inode, type, and none. +The modified field has the aliases date, time, and newest, and its reverse order has the aliases age and oldest. Fields starting with a capital letter will sort uppercase before lowercase: 'A' then 'B' then 'a' then 'b'. Fields starting with a lowercase letter will mix them: 'A' then 'a' then 'B' then 'b'. .RS diff --git a/src/bin/main.rs b/src/bin/main.rs index 0119ee5..7e4a082 100644 --- a/src/bin/main.rs +++ b/src/bin/main.rs @@ -28,7 +28,13 @@ fn main() { }, Err(ref e) if e.is_error() => { - writeln!(stderr(), "{}", e).unwrap(); + let mut stderr = stderr(); + writeln!(stderr, "{}", e).unwrap(); + + if let Some(s) = e.suggestion() { + let _ = writeln!(stderr, "{}", s); + } + exit(exits::OPTIONS_ERROR); }, diff --git a/src/fs/filter.rs b/src/fs/filter.rs index 279c5d1..12d54aa 100644 --- a/src/fs/filter.rs +++ b/src/fs/filter.rs @@ -142,14 +142,14 @@ pub enum SortField { /// files were created on the filesystem, more or less. FileInode, - /// The time this file was modified (the “mtime”). + /// The time the file was modified (the “mtime”). /// /// As this is stored as a Unix timestamp, rather than a local time /// instance, the time zone does not matter and will only be used to /// display the timestamps, not compare them. ModifiedDate, - /// The time file was accessed (the “atime”). + /// The time the was accessed (the “atime”). /// /// Oddly enough, this field rarely holds the *actual* accessed time. /// Recording a read time means writing to the file each time it’s read @@ -159,7 +159,7 @@ pub enum SortField { /// http://unix.stackexchange.com/a/8842 AccessedDate, - /// The time this file was changed or created (the “ctime”). + /// The time the file was changed or created (the “ctime”). /// /// Contrary to the name, this field is used to mark the time when a /// file’s metadata changed -- its permissions, owners, or link count. @@ -173,6 +173,17 @@ pub enum SortField { /// Files are ordered according to the `PartialOrd` implementation of /// `fs::fields::Type`, so changing that will change this. FileType, + + /// The “age” of the file, which is the time it was modified sorted + /// backwards. The reverse of the `ModifiedDate` ordering! + /// + /// It turns out that listing the most-recently-modified files first is a + /// common-enough use case that it deserves its own variant. This would be + /// implemented by just using the modified date and setting the reverse + /// flag, but this would make reversing *that* output not work, which is + /// bad, even though that’s kind of nonsensical. So it’s its own variant + /// that can be reversed like usual. + ModifiedAge, } /// Whether a field should be sorted case-sensitively or case-insensitively. @@ -219,6 +230,7 @@ impl SortField { SortField::ModifiedDate => a.metadata.mtime().cmp(&b.metadata.mtime()), SortField::AccessedDate => a.metadata.atime().cmp(&b.metadata.atime()), SortField::CreatedDate => a.metadata.ctime().cmp(&b.metadata.ctime()), + SortField::ModifiedAge => b.metadata.mtime().cmp(&a.metadata.mtime()), // flip b and a SortField::FileType => match a.type_char().cmp(&b.type_char()) { // todo: this recomputes Ordering::Equal => natord::compare(&*a.name, &*b.name), diff --git a/src/options/filter.rs b/src/options/filter.rs index bfe4da7..6ed2a4e 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. @@ -53,9 +49,19 @@ impl SortField { else if word == "Ext" || word == "Extension" { Ok(SortField::Extension(SortCase::ABCabc)) } - else if word == "mod" || word == "modified" { + else if word == "date" || word == "time" || word == "mod" || word == "modified" || word == "new" || word == "newest" { + // “new” sorts oldest at the top and newest at the bottom; “old” + // sorts newest at the top and oldest at the bottom. I think this + // is the right way round to do this: “size” puts the smallest at + // the top and the largest at the bottom, doesn’t it? Ok(SortField::ModifiedDate) } + else if word == "age" || word == "old" || word == "oldest" { + // Similarly, “age” means that files with the least age (the + // newest files) get sorted at the top, and files with the most + // age (the oldest) at the bottom. + Ok(SortField::ModifiedAge) + } else if word == "acc" || word == "accessed" { Ok(SortField::AccessedDate) } @@ -72,7 +78,7 @@ impl SortField { Ok(SortField::Unsorted) } else { - Err(Misfire::bad_argument(&flags::SORT, word, SORTS)) + Err(Misfire::BadArgument(&flags::SORT, word.into())) } } } @@ -182,12 +188,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] @@ -216,9 +216,14 @@ mod test { test!(one_short: SortField <- ["-saccessed"]; Both => Ok(SortField::AccessedDate)); test!(lowercase: SortField <- ["--sort", "name"]; Both => Ok(SortField::Name(SortCase::AaBbCc))); test!(uppercase: SortField <- ["--sort", "Name"]; Both => Ok(SortField::Name(SortCase::ABCabc))); + test!(old: SortField <- ["--sort", "new"]; Both => Ok(SortField::ModifiedDate)); + test!(oldest: SortField <- ["--sort=newest"]; Both => Ok(SortField::ModifiedDate)); + test!(new: SortField <- ["--sort", "old"]; Both => Ok(SortField::ModifiedAge)); + test!(newest: SortField <- ["--sort=oldest"]; Both => Ok(SortField::ModifiedAge)); + 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/help.rs b/src/options/help.rs index ed56468..e71492c 100644 --- a/src/options/help.rs +++ b/src/options/help.rs @@ -28,7 +28,8 @@ FILTERING AND SORTING OPTIONS --group-directories-first list directories before other files -I, --ignore-glob GLOBS glob patterns (pipe-separated) of files to ignore Valid sort fields: name, Name, extension, Extension, size, type, - modified, accessed, created, inode, none + modified, accessed, created, inode, and none. + date, time, old, and new all refer to modified. "##; static LONG_OPTIONS: &str = r##" diff --git a/src/options/misfire.rs b/src/options/misfire.rs index 1b28179..5cc0ba3 100644 --- a/src/options/misfire.rs +++ b/src/options/misfire.rs @@ -1,23 +1,13 @@ -use std::ffi::{OsStr, OsString}; +use std::ffi::OsString; use std::fmt; use std::num::ParseIntError; use glob; -use options::{HelpString, VersionString}; +use options::{flags, HelpString, VersionString}; use options::parser::{Arg, Flag, ParseError}; -/// A list of legal choices for an argument-taking option -#[derive(PartialEq, Debug)] -pub struct Choices(&'static [&'static str]); - -impl fmt::Display for Choices { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "choices: {}", self.0.join(", ")) - } -} - /// A **misfire** is a thing that can happen instead of listing files -- a /// catch-all for anything outside the program’s normal execution. #[derive(PartialEq, Debug)] @@ -27,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! @@ -70,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<glob::PatternError> for Misfire { @@ -88,10 +70,18 @@ impl From<glob::PatternError> 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), @@ -113,10 +103,43 @@ 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 { + /// Try to second-guess what the user was trying to do, depending on what + /// went wrong. + pub fn suggestion(&self) -> Option<&'static str> { + // ‘ls -lt’ and ‘ls -ltr’ are common combinations + if let Misfire::BadArgument(ref time, ref r) = *self { + if *time == &flags::TIME && r == "r" { + return Some("To sort oldest files last, try \"--sort oldest\", or just \"-sold\""); + } + } + + if let Misfire::InvalidOptions(ParseError::NeedsValue { ref flag, values: _ }) = *self { + if *flag == Flag::Short(b't') { + return Some("To sort newest files last, try \"--sort newest\", or just \"-snew\""); + } + } + + None + } +} + + +/// A list of legal choices for an argument-taking option. +#[derive(PartialEq, Debug)] +pub struct Choices(&'static [&'static str]); + +impl fmt::Display for Choices { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "choices: {}", self.0.join(", ")) + } +} 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<Values>), /// 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<Values> }, - /// 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! // Ov |