update

src/app.rs

@@ -282,7 +282,7 @@ impl App {
             term_width: Term::stdout().size().1 as usize,
             files,
             theme: self.matches.value_of("theme"),
-            line_range: self.matches.value_of("line-range").map(LineRange::from),
+            line_range: transpose(self.matches.value_of("line-range").map(LineRange::from))?,
         })
     }
 
@@ -351,12 +351,12 @@ pub struct LineRange {
 }
 
 impl LineRange {
-    pub fn from(range_raw: &str) -> LineRange {
-        LineRange::parse_range(range_raw).unwrap_or(LineRange::new())
+    pub fn from(range_raw: &str) -> Result<LineRange> {
+        LineRange::parse_range(range_raw)
     }
 
     pub fn new() -> LineRange {
-        LineRange{
+        LineRange {
             lower: usize::min_value(),
             upper: usize::max_value(),
         }
@@ -365,10 +365,10 @@ impl LineRange {
     pub fn parse_range(range_raw: &str) -> Result<LineRange> {
         let mut new_range = LineRange::new();
 
-        if range_raw.bytes().nth(0).ok_or("No first byte")? == b':' {
+        if range_raw.bytes().nth(0).ok_or("Empty line range")? == b':' {
             new_range.upper = range_raw[1..].parse()?;
             return Ok(new_range);
-        } else if range_raw.bytes().last().ok_or("No last byte")? == b':' {
+        } else if range_raw.bytes().last().ok_or("Empty line range")? == b':' {
             new_range.lower = range_raw[..range_raw.len() - 1].parse()?;
             return Ok(new_range);
         }
@@ -377,28 +377,45 @@ impl LineRange {
         if line_numbers.len() == 2 {
             new_range.lower = line_numbers[0].parse()?;
             new_range.upper = line_numbers[1].parse()?;
+            return Ok(new_range);
         }
-        Ok(new_range)                
+        Err("expected single ':' character".into())
     }
 }
 
 #[test]
 fn test_parse_line_range_full() {
-    let range = LineRange::from(Some("40:50")).expect("Shouldn't fail on test!");
+    let range = LineRange::from("40:50").expect("Shouldn't fail on test!");
     assert_eq!(40, range.lower);
     assert_eq!(50, range.upper);
 }
 
 #[test]
 fn test_parse_line_range_partial_min() {
-    let range = LineRange::from(Some(":50")).expect("Shouldn't fail on test!");
+    let range = LineRange::from(":50").expect("Shouldn't fail on test!");
     assert_eq!(usize::min_value(), range.lower);
     assert_eq!(50, range.upper);
 }
 
 #[test]
 fn test_parse_line_range_partial_max() {
-    let range = LineRange::from(Some("40:")).expect("Shouldn't fail on test!");
+    let range = LineRange::from("40:").expect("Shouldn't fail on test!");
     assert_eq!(40, range.lower);
     assert_eq!(usize::max_value(), range.upper);
 }
+
+#[test]
+fn test_parse_line_range_fail() {
+    let range = LineRange::from("40:50:80");
+    assert!(range.is_err());
+    let range = LineRange::from("40::80");
+    assert!(range.is_err());
+    let range = LineRange::from(":40:");
+    assert!(range.is_err());
+    let range = LineRange::from("40");
+    assert!(range.is_err());
+}
+
+fn transpose<T>(opt: Option<Result<T>>) -> Result<Option<T>> {
+    opt.map_or(Ok(None), |res| res.map(Some))
+}

src/features.rs

@@ -1,5 +1,5 @@
 use ansi_term::Colour::Green;
-use app::{Config,LineRange};
+use app::{Config, LineRange};
 use assets::HighlightingAssets;
 use diff::get_git_diff;
 use errors::*;
@@ -94,15 +94,7 @@ fn print_file(
         let highlighter = HighlightLines::new(syntax, theme);
 
         printer.print_header(filename)?;
-
-        match printer.config.line_range.as_ref() {
-            Some(range) => {
-                print_file_ranges(printer, reader, highlighter, range)?;
-            },
-            None => {
-                print_file_no_ranges(printer, reader, highlighter)?;
-            }
-        }
+        print_file_ranges(printer, reader, highlighter, &printer.config.line_range)?;
         printer.print_footer()?;
     }
     Ok(())
@@ -112,8 +104,8 @@ fn print_file_ranges<'a>(
     printer: &mut Printer,
     mut reader: Box<BufRead + 'a>,
     mut highlighter: HighlightLines,
-    ranges: &LineRange
-) -> Result<()>{
+    line_ranges: &Option<LineRange>,
+) -> Result<()> {
     let mut buffer = Vec::new();
 
     while reader.read_until(b'\n', &mut buffer)? > 0 {
@@ -121,36 +113,24 @@ fn print_file_ranges<'a>(
             let line = String::from_utf8_lossy(&buffer);
             let regions = highlighter.highlight(line.as_ref());
 
-            if printer.line_number + 1 < ranges.lower {
-                // skip line
-                printer.line_number += 1;
-            } else if printer.line_number >= ranges.upper {
-                // no more lines in range
-                break;
-            } else {
-                printer.print_line(&regions)?;
+            match line_ranges {
+                &Some(ref range) => {
+                    if printer.line_number + 1 < range.lower {
+                        // skip line
+                        printer.line_number += 1;
+                    } else if printer.line_number >= range.upper {
+                        // no more lines in range
+                        break;
+                    } else {
+                        printer.print_line(&regions)?;
+                    }
+                }
+                &None => {
+                    printer.print_line(&regions)?;
+                }
             }
         }
         buffer.clear();
     }
     Ok(())
 }
-
-fn print_file_no_ranges<'a>(
-    printer: &mut Printer,
-    mut reader: Box<BufRead + 'a>,
-    mut highlighter: HighlightLines
-) -> Result<()>{
-    let mut buffer = Vec::new();
-
-    while reader.read_until(b'\n', &mut buffer)? > 0 {
-        {
-            let line = String::from_utf8_lossy(&buffer);
-            let regions = highlighter.highlight(line.as_ref());
-            printer.print_line(&regions)?;
-        }
-        buffer.clear();
-    }
-
-    Ok(())
-}