Major refactoring and cleanup

src/app.rs

@@ -1,15 +1,51 @@
-use atty::{self, Stream};
-use clap::{App as ClapApp, AppSettings, Arg, ArgGroup, ArgMatches, SubCommand};
-use console::Term;
-use errors::*;
 use std::collections::HashSet;
 use std::env;
-use style::{OutputComponent, OutputComponents, OutputWrap};
+
+use atty::{self, Stream};
+
+use clap::{App as ClapApp, AppSettings, Arg, ArgGroup, ArgMatches, SubCommand};
+
+use console::Term;
 
 #[cfg(windows)]
 use ansi_term;
 
 use assets::BAT_THEME_DEFAULT;
+use errors::*;
+use line_range::LineRange;
+use style::{OutputComponent, OutputComponents, OutputWrap};
+
+#[derive(Debug, Clone, Copy)]
+pub enum PagingMode {
+    Always,
+    QuitIfOneScreen,
+    Never,
+}
+
+pub struct Config<'a> {
+    pub true_color: bool,
+    pub output_wrap: OutputWrap,
+    pub output_components: OutputComponents,
+    pub language: Option<&'a str>,
+    pub colored_output: bool,
+    pub paging_mode: PagingMode,
+    pub term_width: usize,
+    pub files: Vec<Option<&'a str>>,
+    pub theme: String,
+    pub line_range: Option<LineRange>,
+}
+
+fn is_truecolor_terminal() -> bool {
+    env::var("COLORTERM")
+        .map(|colorterm| colorterm == "truecolor" || colorterm == "24bit")
+        .unwrap_or(false)
+}
+
+/// Helper function that should might appear in Rust stable at some point
+/// (https://doc.rust-lang.org/stable/std/option/enum.Option.html#method.transpose)
+fn transpose<T>(opt: Option<Result<T>>) -> Result<Option<T>> {
+    opt.map_or(Ok(None), |res| res.map(Some))
+}
 
 pub struct App {
     pub matches: ArgMatches<'static>,
@@ -319,104 +355,3 @@ impl App {
         }))
     }
 }
-
-#[derive(Debug, Clone, Copy)]
-pub enum PagingMode {
-    Always,
-    QuitIfOneScreen,
-    Never,
-}
-
-pub struct Config<'a> {
-    pub true_color: bool,
-    pub output_wrap: OutputWrap,
-    pub output_components: OutputComponents,
-    pub language: Option<&'a str>,
-    pub colored_output: bool,
-    pub paging_mode: PagingMode,
-    pub term_width: usize,
-    pub files: Vec<Option<&'a str>>,
-    pub theme: String,
-    pub line_range: Option<LineRange>,
-}
-
-fn is_truecolor_terminal() -> bool {
-    env::var("COLORTERM")
-        .map(|colorterm| colorterm == "truecolor" || colorterm == "24bit")
-        .unwrap_or(false)
-}
-
-pub struct LineRange {
-    pub lower: usize,
-    pub upper: usize,
-}
-
-impl LineRange {
-    pub fn from(range_raw: &str) -> Result<LineRange> {
-        LineRange::parse_range(range_raw)
-    }
-
-    pub fn new() -> LineRange {
-        LineRange {
-            lower: usize::min_value(),
-            upper: usize::max_value(),
-        }
-    }
-
-    pub fn parse_range(range_raw: &str) -> Result<LineRange> {
-        let mut new_range = LineRange::new();
-
-        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("Empty line range")? == b':' {
-            new_range.lower = range_raw[..range_raw.len() - 1].parse()?;
-            return Ok(new_range);
-        }
-
-        let line_numbers: Vec<&str> = range_raw.split(':').collect();
-        if line_numbers.len() == 2 {
-            new_range.lower = line_numbers[0].parse()?;
-            new_range.upper = line_numbers[1].parse()?;
-            return Ok(new_range);
-        }
-        Err("expected single ':' character".into())
-    }
-}
-
-#[test]
-fn test_parse_line_range_full() {
-    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(":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("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/assets.rs

@@ -193,7 +193,7 @@ impl HighlightingAssets {
     }
 }
 
-// TODO: ideally, this function would be part of syntect's `ThemeSet`.
+// TODO: this function will soon be part of syntect's `ThemeSet`.
 fn extend_theme_set<P: AsRef<Path>>(theme_set: &mut ThemeSet, folder: P) -> Result<()> {
     let paths = ThemeSet::discover_theme_paths(folder)?;
     for p in &paths {

src/decorations.rs

@@ -14,7 +14,6 @@ pub trait Decoration {
     fn width(&self) -> usize;
 }
 
-// Line number decoration.
 pub struct LineNumberDecoration {
     color: Style,
     cached_wrap: DecorationText,
@@ -65,7 +64,6 @@ impl Decoration for LineNumberDecoration {
     }
 }
 
-// Line changes decoration.
 pub struct LineChangesDecoration {
     cached_none: DecorationText,
     cached_added: DecorationText,
@@ -121,7 +119,6 @@ impl Decoration for LineChangesDecoration {
     }
 }
 
-// Grid border decoration.
 pub struct GridBorderDecoration {
     cached: DecorationText,
 }

src/features.rs

@@ -1,16 +1,20 @@
-use ansi_term::Colour::Green;
-use app::{Config, LineRange};
-use assets::HighlightingAssets;
-use diff::get_git_diff;
-use errors::*;
-use output::OutputType;
-use printer::Printer;
 use std::fs::File;
 use std::io::{self, BufRead, BufReader};
+
+use ansi_term::Colour::Green;
+
 use syntect::easy::HighlightLines;
 use syntect::highlighting::Theme;
 use syntect::parsing::SyntaxDefinition;
 
+use app::Config;
+use assets::HighlightingAssets;
+use diff::get_git_diff;
+use errors::*;
+use line_range::LineRange;
+use output::OutputType;
+use printer::Printer;
+
 pub fn list_languages(assets: &HighlightingAssets, term_width: usize) {
     let mut languages = assets
         .syntax_set
@@ -56,6 +60,13 @@ pub fn list_languages(assets: &HighlightingAssets, term_width: usize) {
     }
 }
 
+pub fn list_themes(assets: &HighlightingAssets) {
+    let themes = &assets.theme_set.themes;
+    for (theme, _) in themes.iter() {
+        println!("{}", theme);
+    }
+}
+
 pub fn print_files(assets: &HighlightingAssets, config: &Config) -> Result<bool> {
     let theme = assets.get_theme(&config.theme);
 
@@ -86,7 +97,7 @@ fn print_file(
     printer: &mut Printer,
     filename: Option<&str>,
 ) -> Result<()> {
-    let stdin = io::stdin(); // TODO: this is not always needed
+    let stdin = io::stdin(); // TODO: this variable is not always needed
     {
         let reader: Box<BufRead> = match filename {
             None => Box::new(stdin.lock()),

src/line_range.rs

@@ -0,0 +1,73 @@
+use errors::*;
+
+pub struct LineRange {
+    pub lower: usize,
+    pub upper: usize,
+}
+
+impl LineRange {
+    pub fn from(range_raw: &str) -> Result<LineRange> {
+        LineRange::parse_range(range_raw)
+    }
+
+    pub fn new() -> LineRange {
+        LineRange {
+            lower: usize::min_value(),
+            upper: usize::max_value(),
+        }
+    }
+
+    pub fn parse_range(range_raw: &str) -> Result<LineRange> {
+        let mut new_range = LineRange::new();
+
+        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("Empty line range")? == b':' {
+            new_range.lower = range_raw[..range_raw.len() - 1].parse()?;
+            return Ok(new_range);
+        }
+
+        let line_numbers: Vec<&str> = range_raw.split(':').collect();
+        if line_numbers.len() == 2 {
+            new_range.lower = line_numbers[0].parse()?;
+            new_range.upper = line_numbers[1].parse()?;
+            return Ok(new_range);
+        }
+
+        Err("expected single ':' character".into())
+    }
+}
+
+#[test]
+fn test_parse_full() {
+    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_partial_min() {
+    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_partial_max() {
+    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_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());
+}

src/main.rs

@@ -22,6 +22,7 @@ mod assets;
 mod decorations;
 mod diff;
 mod features;
+mod line_range;
 mod output;
 mod printer;
 mod style;
@@ -33,7 +34,7 @@ use std::process;
 
 use app::App;
 use assets::{clear_assets, config_dir, HighlightingAssets};
-use features::{list_languages, print_files};
+use features::{list_languages, list_themes, print_files};
 
 mod errors {
     error_chain! {
@@ -62,6 +63,24 @@ mod errors {
 
 use errors::*;
 
+fn run_cache_subcommand(matches: &clap::ArgMatches) -> Result<()> {
+    if matches.is_present("init") {
+        let source_dir = matches.value_of("source").map(Path::new);
+        let target_dir = matches.value_of("target").map(Path::new);
+
+        let blank = matches.is_present("blank");
+
+        let assets = HighlightingAssets::from_files(source_dir, blank)?;
+        assets.save(target_dir)?;
+    } else if matches.is_present("clear") {
+        clear_assets();
+    } else if matches.is_present("config-dir") {
+        println!("{}", config_dir());
+    }
+
+    Ok(())
+}
+
 /// Returns `Err(..)` upon fatal errors. Otherwise, returns `Some(true)` on full success and
 /// `Some(false)` if any intermediate errors occurred (were printed).
 fn run() -> Result<bool> {
@@ -69,21 +88,8 @@ fn run() -> Result<bool> {
 
     match app.matches.subcommand() {
         ("cache", Some(cache_matches)) => {
-            if cache_matches.is_present("init") {
+            run_cache_subcommand(cache_matches)?;
-                let source_dir = cache_matches.value_of("source").map(Path::new);
+            Ok(true)
-                let target_dir = cache_matches.value_of("target").map(Path::new);
-
-                let blank = cache_matches.is_present("blank");
-
-                let assets = HighlightingAssets::from_files(source_dir, blank)?;
-                assets.save(target_dir)?;
-            } else if cache_matches.is_present("clear") {
-                clear_assets();
-            } else if cache_matches.is_present("config-dir") {
-                println!("{}", config_dir());
-            }
-
-            return Ok(true);
         }
         _ => {
             let config = app.config()?;
@@ -91,21 +97,18 @@ fn run() -> Result<bool> {
 
             if app.matches.is_present("list-languages") {
                 list_languages(&assets, config.term_width);
-                return Ok(true);
-            }
 
-            if app.matches.is_present("list-themes") {
+                Ok(true)
-                let themes = &assets.theme_set.themes;
+            } else if app.matches.is_present("list-themes") {
-                for (theme, _) in themes.iter() {
+                list_themes(&assets);
-                    println!("{}", theme);
-                }
-                return Ok(true);
-            }
 
+                Ok(true)
+            } else {
                 print_files(&assets, &config)
             }
         }
     }
+}
 
 fn main() {
     let result = run();

src/output.rs

@@ -1,9 +1,10 @@
-use app::PagingMode;
-use errors::*;
 use std::env;
 use std::io::{self, Write};
 use std::process::{Child, Command, Stdio};
 
+use app::PagingMode;
+use errors::*;
+
 pub enum OutputType {
     Pager(Child),
     Stdout(io::Stdout),

src/printer.rs

@@ -1,15 +1,19 @@
-use ansi_term::Colour::{Fixed, Green, Red, Yellow};
-use ansi_term::Style;
-use app::Config;
-use console::AnsiCodeIterator;
-use decorations::{Decoration, GridBorderDecoration, LineChangesDecoration, LineNumberDecoration};
-use diff::LineChanges;
-use errors::*;
 use std::boxed::Box;
 use std::io::Write;
 use std::vec::Vec;
-use style::OutputWrap;
+
+use ansi_term::Colour::{Fixed, Green, Red, Yellow};
+use ansi_term::Style;
+
+use console::AnsiCodeIterator;
+
 use syntect::highlighting::{self, Theme};
+
+use app::Config;
+use decorations::{Decoration, GridBorderDecoration, LineChangesDecoration, LineNumberDecoration};
+use diff::LineChanges;
+use errors::*;
+use style::OutputWrap;
 use terminal::{as_terminal_escaped, to_ansi_color};
 
 pub struct Printer<'a> {

src/style.rs

@@ -1,7 +1,8 @@
-use errors::*;
 use std::collections::HashSet;
 use std::str::FromStr;
 
+use errors::*;
+
 #[derive(Debug, Eq, PartialEq, Copy, Clone, Hash)]
 pub enum OutputComponent {
     Auto,

src/terminal.rs

@@ -1,5 +1,6 @@
 use ansi_term::Colour::{Fixed, RGB};
 use ansi_term::{self, Style};
+
 use syntect::highlighting::{self, FontStyle};
 
 /// Approximate a 24 bit color value by a 8 bit ANSI code