Merge branch 'master' into optional_args

.travis.yml

@@ -8,4 +8,6 @@ rust:
   - stable
 script:
     - cargo build --verbose
-    - cargo test --verbose
+    - cargo test  --verbose
+    - cargo build --verbose --no-default-features
+    - cargo test  --verbose --no-default-features

README.md

@@ -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**.
 

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
             ;;
 

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'
 "
 

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]" \

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

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);
         },
 

src/fs/feature/mod.rs

@@ -10,7 +10,7 @@ pub mod git {
     use std::iter::FromIterator;
     use std::path::{Path, PathBuf};
 
-    use fs::fields;
+    use fs::fields as f;
 
 
     pub struct GitCache;
@@ -22,20 +22,12 @@ pub mod git {
     }
 
     impl GitCache {
-        pub fn get(&self, _index: &Path) -> Option<Git> {
+        pub fn has_anything_for(&self, _index: &Path) -> bool {
+            false
+        }
+
+        pub fn get(&self, _index: &Path, _prefix_lookup: bool) -> f::Git {
             panic!("Tried to query a Git cache, but Git support is disabled")
         }
     }
-
-    pub struct Git;
-
-    impl Git {
-        pub fn status(&self, _: &Path) -> fields::Git {
-            panic!("Tried to get a Git status, but Git support is disabled")
-        }
-
-        pub fn dir_status(&self, path: &Path) -> fields::Git {
-            self.status(path)
-        }
-    }
 }

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),

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));

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 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 };
+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 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 };
+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 };

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##"

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 value {:?} (Choices: {})", 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),
+            NeedsValue { ref flag, values: None }     => write!(f, "Flag {} needs a value", flag),
-            ForbiddenValue { ref flag }          => write!(f, "Flag {} cannot take a value", flag),
+            NeedsValue { ref flag, values: Some(cs) } => write!(f, "Flag {} needs a value ({})", flag, Choices(cs)),
-            UnknownShortArgument { ref attempt } => write!(f, "Unknown argument -{}", *attempt as char),
+            ForbiddenValue { ref flag }               => write!(f, "Flag {} cannot take a value", flag),
-            UnknownArgument { ref attempt }      => write!(f, "Unknown argument --{}", attempt.to_string_lossy()),
+            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(", "))
+    }
+}

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,13 +95,15 @@ 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,
 
     /// This flag may be followed by a value to override its defaults
-    Optional,
+    Optional(Option<Values>),
 }
 
 
@@ -174,7 +183,7 @@ impl Args {
                     let arg = self.lookup_long(before)?;
                     let flag = Flag::Long(arg.long);
                     match arg.takes_value {
-                        Necessary|Optional  => result_flags.push((flag, Some(after))),
+                        Necessary(_)|Optional(_)  => result_flags.push((flag, Some(after))),
                         Forbidden  => return Err(ParseError::ForbiddenValue { flag })
                     }
                 }
@@ -185,16 +194,16 @@ 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)),
+                        Forbidden         => result_flags.push((flag, None)),
-                        Necessary  => {
+                        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 })
                             }
                         },
-                        Optional => {
+                        Optional(_) => {
                             if let Some(next_arg) = inputs.next() {
                                 result_flags.push((flag, Some(next_arg)));
                             }
@@ -221,7 +230,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();
@@ -231,8 +240,8 @@ impl Args {
                         let arg = self.lookup_short(*byte)?;
                         let flag = Flag::Short(*byte);
                         match arg.takes_value {
-                            Forbidden|Optional  => result_flags.push((flag, None)),
+                            Forbidden|Optional(_)  => result_flags.push((flag, None)),
-                            Necessary  => return Err(ParseError::NeedsValue { flag })
+                            Necessary(values)  => return Err(ParseError::NeedsValue { flag, values })
                         }
                     }
 
@@ -240,7 +249,7 @@ impl Args {
                     let arg = self.lookup_short(*arg_with_value)?;
                     let flag = Flag::Short(arg.short.unwrap());
                     match arg.takes_value {
-                        Necessary|Optional  => result_flags.push((flag, Some(after))),
+                        Necessary(_)|Optional(_)  => result_flags.push((flag, Some(after))),
                         Forbidden  => return Err(ParseError::ForbiddenValue { flag })
                     }
                 }
@@ -248,7 +257,7 @@ impl Args {
                 // 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’
@@ -262,8 +271,8 @@ impl Args {
                         let arg = self.lookup_short(*byte)?;
                         let flag = Flag::Short(*byte);
                         match arg.takes_value {
-                            Forbidden  => result_flags.push((flag, None)),
+                            Forbidden         => result_flags.push((flag, None)),
-                            Necessary|Optional  => {
+                            Necessary(values)|Optional(values) => {
                                 if index < bytes.len() - 1 {
                                     let remnants = &bytes[index+1 ..];
                                     result_flags.push((flag, Some(OsStr::from_bytes(remnants))));
@@ -275,10 +284,10 @@ impl Args {
                                 else {
                                     match arg.takes_value {
                                         Forbidden => assert!(false),
-                                        Necessary => {
+                                        Necessary(_) => {
-                                            return Err(ParseError::NeedsValue { flag });
+                                            return Err(ParseError::NeedsValue { flag, values });
                                         },
-                                        Optional => {
+                                        Optional(_) => {
                                             result_flags.push((flag, None));
                                         }
                                     }
@@ -386,7 +395,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.
@@ -427,15 +436,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
@@ -563,10 +572,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)) }
     ];
 
 
@@ -589,10 +601,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) ]);
@@ -602,13 +619,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") });
@@ -639,7 +662,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);

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!(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::bad_argument(&flags::COLOR, &os("lovers"),   super::COLOURS));  // and so is this one!
+    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));

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_tea:  TimeTypes <- ["--time=tea"];          Both => err Misfire::BadArgument(&flags::TIME, OsString::from("tea")));
-        test!(time_ea:   TimeTypes <- ["-tea"];                Both => err Misfire::bad_argument(&flags::TIME, &os("ea"), super::TIMES));
+        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 }));

src/output/details.rs

@@ -203,23 +203,24 @@ impl<'a> Render<'a> {
                     // There are three “levels” of extended attribute support:
                     //
                     // 1. If we’re compiling without that feature, then
-                    //    exa pretends no files have attributes.
+                    //    exa pretends all files have no attributes.
-                    // 2. If the feature is enabled but the --extended flag
+                    // 2. If the feature is enabled and the --extended flag
-                    //    hasn’t been specified, then display an @ in the
+                    //    has been specified, then display an @ in the
-                    //    permissions column for files with xattrs, but don’t
+                    //    permissions column for files with attributes, the
-                    //    display anything else.
+                    //    names of all attributes and their lengths, and any
-                    // 3. If the --extended flag *has* been specified, then
+                    //    errors encountered when getting them.
-                    //    display the @, the attributes and their lengths,
+                    // 3. If the --extended flag *hasn’t* been specified, then
-                    //    and any errors encountered when getting them.
+                    //    display the @, but don’t display anything else.
                     //
-                    // For a while, exa took a stricter approach to (2): if
+                    // For a while, exa took a stricter approach to (3):
-                    // an error occurred while checking a file’s xattrs, exa
+                    // if an error occurred while checking a file’s xattrs to
-                    // would display that error even though the attributes
+                    // see if it should display the @, exa would display that
-                    // weren’t actually being shown! This was confusing, as
+                    // error even though the attributes weren’t actually being
-                    // users were being shown errors for something they didn’t
+                    // shown! This was confusing, as users were being shown
-                    // explicitly ask for, and just cluttered up the output.
+                    // errors for something they didn’t explicitly ask for,
-                    // So now errors aren’t printed unless the user passes
+                    // and just cluttered up the output. So now errors aren’t
-                    // --extended to signify that they want to see them.
+                    // printed unless the user passes --extended to signify
+                    // that they want to see them.
 
                     if xattr::ENABLED {
                         match file.path.attributes() {

xtests/dates_deifidom

@@ -0,0 +1,4 @@
+Permissions Size User      Date Modified Name
+.rw-rw-r--     0 cassowary 22 Dec  2009  plum
+.rw-rw-r--     0 cassowary 15 Jun  2006  peach
+.rw-rw-r--     0 cassowary  3 Mar  2003  pear

xtests/error_lt

@@ -0,0 +1,2 @@
+Flag -t needs a value (choices: modified, accessed, created)
+To sort newest files last, try "--sort newest", or just "-snew"

xtests/error_ltr

@@ -0,0 +1,2 @@
+Option --time (-t) has no "r" setting (choices: modified, accessed, created)
+To sort oldest files last, try "--sort oldest", or just "-sold"

xtests/error_setting

@@ -0,0 +1 @@
+Option --time-style has no "24" setting (choices: default, long-iso, full-iso, iso)

xtests/error_value

@@ -1 +1 @@
-Flag --time needs a value
+Flag --time needs a value (choices: modified, accessed, created)

xtests/help

@@ -23,7 +23,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.
 
 LONG VIEW OPTIONS
   -b, --binary       list file sizes with binary prefixes

xtests/run.sh

@@ -143,9 +143,12 @@ $exa $testcases/file-names-exts/music.* -I "*.OGG|*.mp3" -1 2>&1 | diff -q - $re
 # Dates and times
 $exa $testcases/dates -lh --accessed --sort=accessed 2>&1 | diff -q - $results/dates_accessed  || exit 1
 $exa $testcases/dates -lh            --sort=modified 2>&1 | diff -q - $results/dates_modified  || exit 1
+$exa $testcases/dates -lh            --sort=newest   2>&1 | diff -q - $results/dates_modified  || exit 1
+$exa $testcases/dates -lh         -r --sort=newest   2>&1 | diff -q - $results/dates_deifidom  || exit 1
+$exa $testcases/dates -lh            --sort=oldest   2>&1 | diff -q - $results/dates_deifidom  || exit 1
 $exa $testcases/dates -l       --time-style=long-iso 2>&1 | diff -q - $results/dates_long_iso  || exit 1
 $exa $testcases/dates -l       --time-style=full-iso 2>&1 | diff -q - $results/dates_full_iso  || exit 1
-$exa $testcases/dates -l            --time-style=iso 2>&1 | diff -q - $results/dates_iso       || exit 1
+$exa $testcases/dates -l       --time-style=iso      2>&1 | diff -q - $results/dates_iso       || exit 1
 
 # Locales
 # These two are used in particular because they have 5-long and 4-long
@@ -252,13 +255,18 @@ EXA_COLORS="di=38;5;195:fi=38;5;250:xx=38;5;237:ur=38;5;194:uw=38;5;193:ux=38;5;
 EXA_COLORS="reset" $exa $testcases/file-names-exts -1  2>&1 | diff -q - $results/themed_un  || exit 1
 
 # Errors
-$exa --binary     2>&1 | diff -q - $results/error_useless     || exit 1
+$exa --binary           2>&1 | diff -q - $results/error_useless     || exit 1
-$exa --ternary    2>&1 | diff -q - $results/error_long        || exit 1
+$exa --ternary          2>&1 | diff -q - $results/error_long        || exit 1
-$exa -4           2>&1 | diff -q - $results/error_short       || exit 1
+$exa -4                 2>&1 | diff -q - $results/error_short       || exit 1
-$exa --time       2>&1 | diff -q - $results/error_value       || exit 1
+$exa --time             2>&1 | diff -q - $results/error_value       || exit 1
-$exa --long=time  2>&1 | diff -q - $results/error_overvalued  || exit 1
+$exa --long=time        2>&1 | diff -q - $results/error_overvalued  || exit 1
-$exa -l --long    2>&1 | diff -q - $results/error_duplicate   || exit 1
+$exa -l --long          2>&1 | diff -q - $results/error_duplicate   || exit 1
-$exa -ll          2>&1 | diff -q - $results/error_twice       || exit 1
+$exa -ll                2>&1 | diff -q - $results/error_twice       || exit 1
+$exa -l --time-style=24 2>&1 | diff -q - $results/error_setting     || exit 1
+
+# Error suggestions
+$exa -ltr 2>&1 | diff -q - $results/error_ltr  || exit 1
+$exa -lt  2>&1 | diff -q - $results/error_lt   || exit 1
 
 
 # Debug mode