Give IgnorePatterns a better interface

This commit gives IgnorePatterns a bunch of constructor methods that mean its option-parsing sister file doesn’t need to know that it’s a vec of glob patterns inside: it can work with anything that iterates over strings. Now, the options module doesn’t need to know about the glob crate.

src/fs/filter.rs

@@ -1,4 +1,5 @@
 use std::cmp::Ordering;
+use std::iter::FromIterator;
 use std::os::unix::fs::MetadataExt;
 
 use glob;
@@ -209,12 +210,45 @@ pub enum SortCase {
 }
 
 
+/// The **ignore patterns** are a list of globs that are tested against
+/// each filename, and if any of them match, that file isn’t displayed.
+/// This lets a user hide, say, text files by ignoring `*.txt`.
 #[derive(PartialEq, Default, Debug, Clone)]
 pub struct IgnorePatterns {
-    pub patterns: Vec<glob::Pattern>,
+    patterns: Vec<glob::Pattern>,
+}
+
+impl FromIterator<glob::Pattern> for IgnorePatterns {
+    fn from_iter<I: IntoIterator<Item = glob::Pattern>>(iter: I) -> Self {
+        IgnorePatterns { patterns: iter.into_iter().collect() }
+    }
 }
 
 impl IgnorePatterns {
+
+    /// Create a new list from the input glob strings, turning the inputs that
+    /// are valid glob patterns into an IgnorePatterns. The inputs that don’t
+    /// parse correctly are returned separately.
+    pub fn parse_from_iter<'a, I: IntoIterator<Item = &'a str>>(iter: I) -> (Self, Vec<glob::PatternError>) {
+        let mut patterns = Vec::new();
+        let mut errors = Vec::new();
+
+        for input in iter {
+            match glob::Pattern::new(input) {
+                Ok(pat) => patterns.push(pat),
+                Err(e)  => errors.push(e),
+            }
+        }
+
+        (IgnorePatterns { patterns }, errors)
+    }
+
+    /// Create a new empty list that matches nothing.
+    pub fn empty() -> IgnorePatterns {
+        IgnorePatterns { patterns: Vec::new() }
+    }
+
+    /// Test whether the given file should be hidden from the results.
     fn is_ignored(&self, file: &File) -> bool {
         self.patterns.iter().any(|p| p.matches(&file.name))
     }

src/options/filter.rs

@@ -1,5 +1,3 @@
-use glob;
-
 use fs::DotFilter;
 use fs::filter::{FileFilter, SortField, SortCase, IgnorePatterns};
 
@@ -102,15 +100,22 @@ impl IgnorePatterns {
     /// Determines the set of file filter options to use, based on the user’s
     /// command-line arguments.
     pub fn deduce(matches: &MatchedFlags) -> Result<IgnorePatterns, Misfire> {
-        let patterns = match matches.get(&flags::IGNORE_GLOB) {
-            None => Ok(Vec::new()),
-            Some(is) => is.to_string_lossy().split('|').map(|a| glob::Pattern::new(a)).collect(),
-        }?;
 
-        // TODO: is to_string_lossy really the best way to handle
-        // invalid UTF-8 there?
+        let inputs = match matches.get(&flags::IGNORE_GLOB) {
+            None => return Ok(IgnorePatterns::empty()),
+            Some(is) => is,
+        };
+
+        let (patterns, mut errors) = IgnorePatterns::parse_from_iter(inputs.to_string_lossy().split('|'));
 
-        Ok(IgnorePatterns { patterns })
+        // It can actually return more than one glob error,
+        // but we only use one.
+        if let Some(error) = errors.pop() {
+            return Err(error.into())
+        }
+        else {
+            Ok(patterns)
+        }
     }
 }
 
@@ -185,6 +190,7 @@ mod test {
 
     mod ignore_patternses {
         use super::*;
+        use std::iter::FromIterator;
         use glob;
 
         fn pat(string: &'static str) -> glob::Pattern {
@@ -192,9 +198,9 @@ mod test {
         }
 
         // Various numbers of globs
-        test!(none:   IgnorePatterns <- []                             => Ok(IgnorePatterns { patterns: vec![] }));
-        test!(one:    IgnorePatterns <- ["--ignore-glob", "*.ogg"]     => Ok(IgnorePatterns { patterns: vec![ pat("*.ogg") ] }));
-        test!(two:    IgnorePatterns <- ["--ignore-glob=*.ogg|*.MP3"]  => Ok(IgnorePatterns { patterns: vec![ pat("*.ogg"), pat("*.MP3") ] }));
-        test!(loads:  IgnorePatterns <- ["-I*|?|.|*"]  => Ok(IgnorePatterns { patterns: vec![ pat("*"), pat("?"), pat("."), pat("*") ] }));
+        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("*") ])));
     }
 }