Repository: casey/just
Pull request #204 (2017-05-20 18:27:26)

Refactor color handling
Color logic is fairly complicated, so moved it into its own
module.

A `Color` object now encapsulates the --color setting, which
stream we are printing to, and what color we are painting.

This way, Color::paint can just do the right thing when asked to
paint text.

Also added tests to make sure that --list and --highlight colors
are using the correct color codes.


Patch:
diff --git a/src/app.rs b/src/app.rs
index 4914fa8e..867f5f24 100644
--- a/src/app.rs
+++ b/src/app.rs
@@ -1,9 +1,8 @@
-extern crate ansi_term;
-extern crate atty;
 extern crate clap;
 extern crate libc;
 
-use ::prelude::*;
+use color::Color;
+use prelude::*;
 use std::{convert, ffi};
 use std::collections::BTreeMap;
 use self::clap::{App, Arg, ArgGroup, AppSettings};
@@ -25,54 +24,6 @@ macro_rules! die {
   }};
 }
 
-#[derive(Copy, Clone)]
-pub enum UseColor {
-  Auto,
-  Always,
-  Never,
-}
-
-impl Default for UseColor {
-  fn default() -> UseColor {
-    UseColor::Never
-  }
-}
-
-impl UseColor {
-  fn from_argument(use_color: &str) -> Option<UseColor> {
-    match use_color {
-      "auto"   => Some(UseColor::Auto),
-      "always" => Some(UseColor::Always),
-      "never"  => Some(UseColor::Never),
-      _        => None,
-    }
-  }
-
-  fn should_color_stream(self, stream: atty::Stream) -> bool {
-    match self {
-      UseColor::Auto   => atty::is(stream),
-      UseColor::Always => true,
-      UseColor::Never  => false,
-    }
-  }
-
-  pub fn should_color_stdout(self) -> bool {
-    self.should_color_stream(atty::Stream::Stdout)
-  }
-
-  pub fn should_color_stderr(self) -> bool {
-    self.should_color_stream(atty::Stream::Stderr)
-  }
-
-  fn blue(self, stream: atty::Stream) -> ansi_term::Style {
-    if self.should_color_stream(stream) {
-      ansi_term::Style::new().fg(ansi_term::Color::Blue)
-    } else {
-      ansi_term::Style::default()
-    }
-  }
-}
-
 fn edit<P: convert::AsRef<ffi::OsStr>>(path: P) -> ! {
   let editor = env::var_os("EDITOR")
     .unwrap_or_else(|| die!("Error getting EDITOR environment variable"));
@@ -167,10 +118,11 @@ pub fn app() {
          .args(&["DUMP", "EDIT", "LIST", "SHOW", "SUMMARY", "ARGUMENTS", "EVALUATE"]))
     .get_matches();
 
-  let use_color_argument = matches.value_of("COLOR").expect("--color had no value");
-  let use_color = match UseColor::from_argument(use_color_argument) {
-    Some(use_color) => use_color,
-    None => die!("Invalid argument to --color. This is a bug in just."),
+  let color = match matches.value_of("COLOR").expect("`--color` had no value") {
+    "auto"   => Color::auto(),
+    "always" => Color::always(),
+    "never"  => Color::never(),
+    other    => die!("Invalid argument `{}` to --color. This is a bug in just.", other),
   };
 
   let set_count = matches.occurrences_of("SET");
@@ -274,7 +226,7 @@ pub fn app() {
   }
 
   let justfile = compile(&text).unwrap_or_else(|error|
-    if use_color.should_color_stderr() {
+    if color.stderr().active() {
       die!("{:#}", error);
     } else {
       die!("{}", error);
@@ -296,19 +248,19 @@ pub fn app() {
   }
 
   if matches.is_present("LIST") {
-    let blue = use_color.blue(atty::Stream::Stdout);
+    let doc_color = color.stdout().doc();
     println!("Available recipes:");
     for (name, recipe) in &justfile.recipes {
       print!("    {}", name);
       for parameter in &recipe.parameters {
-        if use_color.should_color_stdout() {
+        if color.stdout().active() {
           print!(" {:#}", parameter);
         } else {
           print!(" {}", parameter);
         }
       }
       if let Some(doc) = recipe.doc {
-        print!(" {} {}", blue.paint("#"), blue.paint(doc));
+        print!(" {} {}", doc_color.paint("#"), doc_color.paint(doc));
       }
       println!("");
     }
@@ -351,13 +303,13 @@ pub fn app() {
     overrides: overrides,
     quiet:     matches.is_present("QUIET"),
     shell:     matches.value_of("SHELL"),
-    use_color: use_color,
+    color:     color,
     verbose:   matches.is_present("VERBOSE"),
   };
 
   if let Err(run_error) = justfile.run(&arguments, &options) {
     if !options.quiet {
-      if use_color.should_color_stderr() {
+      if color.stderr().active() {
         warn!("{:#}", run_error);
       } else {
         warn!("{}", run_error);
diff --git a/src/color.rs b/src/color.rs
new file mode 100644
index 00000000..9b891446
--- /dev/null
+++ b/src/color.rs
@@ -0,0 +1,145 @@
+extern crate ansi_term;
+extern crate atty;
+
+use prelude::*;
+use self::ansi_term::{Style, Prefix, Suffix, ANSIGenericString};
+use self::ansi_term::Color::*;
+use self::atty::is as is_atty;
+use self::atty::Stream;
+
+#[derive(Copy, Clone)]
+pub enum UseColor {
+  Auto,
+  Always,
+  Never,
+}
+
+#[derive(Copy, Clone)]
+pub struct Color {
+  use_color: UseColor,
+  atty:      bool,
+  style:     Style,
+}
+
+impl Default for Color {
+  fn default() -> Color {
+    Color {
+      use_color: UseColor::Never,
+      atty:      false,
+      style:     Style::new(),
+    }
+  }
+}
+
+impl Color {
+  fn restyle(self, style: Style) -> Color {
+    Color {
+      style: style,
+      ..self
+    }
+  }
+
+  fn redirect(self, stream: Stream) -> Color {
+    Color {
+      atty: is_atty(stream),
+      ..self
+    }
+  }
+
+  fn effective_style(&self) -> Style {
+    if self.active() {
+      self.style
+    } else {
+      Style::new()
+    }
+  }
+
+  pub fn fmt(fmt: &fmt::Formatter) -> Color {
+    if fmt.alternate() {
+      Color::always()
+    } else {
+      Color::never()
+    }
+  }
+
+  pub fn auto() -> Color {
+    Color {
+      use_color: UseColor::Auto,
+      ..default()
+    }
+  }
+
+  pub fn always() -> Color {
+    Color {
+      use_color: UseColor::Always,
+      ..default()
+    }
+  }
+
+  pub fn never() -> Color {
+    Color {
+      use_color: UseColor::Never,
+      ..default()
+    }
+  }
+
+  pub fn stderr(self) -> Color {
+    self.redirect(Stream::Stderr)
+  }
+
+  pub fn stdout(self) -> Color {
+    self.redirect(Stream::Stdout)
+  }
+
+  pub fn doc(self) -> Color {
+    self.restyle(Style::new().fg(Blue))
+  }
+
+  pub fn error(self) -> Color {
+    self.restyle(Style::new().fg(Red).bold())
+  }
+
+  pub fn banner(self) -> Color {
+    self.restyle(Style::new().fg(Cyan).bold())
+  }
+
+  pub fn command(self) -> Color {
+    self.restyle(Style::new().bold())
+  }
+
+  pub fn parameter(self) -> Color {
+    self.restyle(Style::new().fg(Cyan))
+  }
+
+  pub fn message(self) -> Color {
+    self.restyle(Style::new().bold())
+  }
+
+  pub fn annotation(self) -> Color {
+    self.restyle(Style::new().fg(Purple))
+  }
+
+  pub fn string(self) -> Color {
+    self.restyle(Style::new().fg(Green))
+  }
+
+  pub fn active(&self) -> bool {
+    match self.use_color {
+      UseColor::Always => true,
+      UseColor::Never  => false,
+      UseColor::Auto   => self.atty,
+    }
+  }
+
+  pub fn paint<'a>(&self, text: &'a str) -> ANSIGenericString<'a, str> {
+    self.effective_style().paint(text)
+  }
+
+  pub fn prefix(&self) -> Prefix {
+    self.effective_style().prefix()
+  }
+
+  pub fn suffix(&self) -> Suffix {
+    self.effective_style().suffix()
+  }
+}
diff --git a/src/integration.rs b/src/integration.rs
index c13d4d99..5c9d2c84 100644
--- a/src/integration.rs
+++ b/src/integration.rs
@@ -1586,3 +1586,29 @@ a: x y
 ",
   status:   EXIT_FAILURE,
 }
+
+integration_test! {
+  name:     list_colors,
+  justfile: "
+# comment
+a B C +D='hello':
+  echo {{B}} {{C}} {{D}}
+",
+  args:     ("--color", "always", "--list"),
+  stdout:   "Available recipes:\n    a \u{1b}[36mB\u{1b}[0m \u{1b}[36mC\u{1b}[0m \u{1b}[35m+\u{1b}[0m\u{1b}[36mD\u{1b}[0m=\'\u{1b}[32mhello\u{1b}[0m\' \u{1b}[34m#\u{1b}[0m \u{1b}[34mcomment\u{1b}[0m\n",
+  stderr:   "",
+  status:   EXIT_SUCCESS,
+}
+
+integration_test! {
+  name:     run_colors,
+  justfile: "
+# comment
+a:
+  echo hi
+",
+  args:     ("--color", "always", "--highlight", "--verbose"),
+  stdout:   "hi\n",
+  stderr:   "\u{1b}[1;36m===> Running recipe `a`...\u{1b}[0m\n\u{1b}[1mecho hi\u{1b}[0m\n",
+  status:   EXIT_SUCCESS,
+}
diff --git a/src/lib.rs b/src/lib.rs
index ea4c361e..da585c9c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -25,20 +25,26 @@ mod platform;
 
 mod app;
 
+mod color;
+
 mod prelude {
-  pub use std::io::prelude::*;
   pub use libc::{EXIT_FAILURE, EXIT_SUCCESS};
   pub use regex::Regex;
+  pub use std::io::prelude::*;
   pub use std::path::{Path, PathBuf};
   pub use std::{cmp, env, fs, fmt, io, iter, process};
+
+  pub fn default<T: Default>() -> T {
+    Default::default()
+  }
 }
 
 use prelude::*;
 
 pub use app::app;
 
-use app::UseColor;
 use brev::{output, OutputError};
+use color::Color;
 use platform::{Platform, PlatformInterface};
 use std::borrow::Cow;
 use std::collections::{BTreeMap as Map, BTreeSet as Set};
@@ -129,16 +135,14 @@ struct Parameter<'a> {
 
 impl<'a> Display for Parameter<'a> {
   fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-    let green = maybe_green(f.alternate());
-    let cyan = maybe_cyan(f.alternate());
-    let purple = maybe_purple(f.alternate());
+    let color = Color::fmt(f);
     if self.variadic {
-      write!(f, "{}", purple.paint("+"))?;
+      write!(f, "{}", color.annotation().paint("+"))?;
     }
-    write!(f, "{}", cyan.paint(self.name))?;
+    write!(f, "{}", color.parameter().paint(self.name))?;
     if let Some(ref default) = self.default {
       let escaped = default.chars().flat_map(char::escape_default).collect::<String>();;
-      write!(f, r#"='{}'"#, green.paint(escaped))?;
+      write!(f, r#"='{}'"#, color.string().paint(&escaped))?;
     }
     Ok(())
   }
@@ -285,8 +289,8 @@ impl<'a> Recipe<'a> {
     options:   &RunOptions,
   ) -> Result<(), RunError<'a>> {
     if options.verbose {
-      let cyan = maybe_cyan(options.use_color.should_color_stderr());
-      warn!("{}===> Running recipe `{}`...{}", cyan.prefix(), self.name, cyan.suffix());
+      let color = options.color.stderr().banner();
+      warn!("{}===> Running recipe `{}`...{}", color.prefix(), self.name, color.suffix());
     }
 
     let mut argument_map = Map::new();
@@ -430,12 +434,13 @@ impl<'a> Recipe<'a> {
           continue;
         }
 
-        if options.dry_run
-          || options.verbose
-          || !((quiet_command ^ self.quiet) || options.quiet) {
-          let highlight = maybe_highlight(options.highlight
-                                          && options.use_color.should_color_stderr());
-          warn!("{}", highlight.paint(command));
+        if options.dry_run || options.verbose || !((quiet_command ^ self.quiet) || options.quiet) {
+          let color = if options.highlight {
+            options.color.command()
+          } else {
+            options.color
+          };
+          warn!("{}", color.stderr().paint(command));
         }
 
         if options.dry_run {
@@ -946,7 +951,7 @@ fn write_error_context(
   width:  Option<usize>,
 ) -> Result<(), fmt::Error> {
   let line_number = line + 1;
-  let red = maybe_red(f.alternate());
+  let red = Color::fmt(f).error();
   match text.lines().nth(line) {
     Some(line) => {
       let mut i = 0;
@@ -1003,61 +1008,13 @@ fn write_token_error_context(f: &mut fmt::Formatter, token: &Token) -> Result<()
   )
 }
 
-fn maybe_red(colors: bool) -> ansi_term::Style {
-  if colors {
-    ansi_term::Style::new().fg(ansi_term::Color::Red).bold()
-  } else {
-    ansi_term::Style::default()
-  }
-}
-
-fn maybe_green(colors: bool) -> ansi_term::Style {
-  if colors {
-    ansi_term::Style::new().fg(ansi_term::Color::Green)
-  } else {
-    ansi_term::Style::default()
-  }
-}
-
-fn maybe_cyan(colors: bool) -> ansi_term::Style {
-  if colors {
-    ansi_term::Style::new().fg(ansi_term::Color::Cyan)
-  } else {
-    ansi_term::Style::default()
-  }
-}
-
-fn maybe_purple(colors: bool) -> ansi_term::Style {
-  if colors {
-    ansi_term::Style::new().fg(ansi_term::Color::Purple)
-  } else {
-    ansi_term::Style::default()
-  }
-}
-
-fn maybe_bold(colors: bool) -> ansi_term::Style {
-  if colors {
-    ansi_term::Style::new().bold()
-  } else {
-    ansi_term::Style::default()
-  }
-}
-
-fn maybe_highlight(colors: bool) -> ansi_term::Style {
-  if colors {
-    ansi_term::Style::new().fg(ansi_term::Color::Cyan).bold()
-  } else {
-    ansi_term::Style::default()
-  }
-}
-
 impl<'a> Display for CompileError<'a> {
   fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
     use ErrorKind::*;
-    let red  = maybe_red(f.alternate());
-    let bold = maybe_bold(f.alternate());
+    let error   = Color::fmt(f).error();
+    let message = Color::fmt(f).message();
 
-    write!(f, "{} {}", red.paint("error:"), bold.prefix())?;
+    write!(f, "{} {}", error.paint("error:"), message.prefix())?;
 
     match self.kind {
       CircularRecipeDependency{recipe, ref circle} => {
@@ -1148,7 +1105,7 @@ impl<'a> Display for CompileError<'a> {
       }
     }
 
-    write!(f, "{}", bold.suffix())?;
+    write!(f, "{}", message.suffix())?;
 
     write_error_context(f, self.text, self.index, self.line, self.column, self.width)
   }
@@ -1168,7 +1125,7 @@ struct RunOptions<'a> {
   overrides: Map<&'a str, &'a str>,
   quiet:     bool,
   shell:     Option<&'a str>,
-  use_color: UseColor,
+  color:     Color,
   verbose:   bool,
 }
 
@@ -1347,9 +1304,10 @@ impl<'a> RunError<'a> {
 impl<'a> Display for RunError<'a> {
   fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
     use RunError::*;
-    let red = maybe_red(f.alternate());
-    let bold = maybe_bold(f.alternate());
-    write!(f, "{} {}", red.paint("error:"), bold.prefix())?;
+    let color = if f.alternate() { Color::always() } else { Color::never() };
+    let error = color.error();
+    let message = color.message();
+    write!(f, "{} {}", error.paint("error:"), message.prefix())?;
 
     let mut error_token = None;
 
@@ -1491,7 +1449,7 @@ impl<'a> Display for RunError<'a> {
       }
     }
 
-    write!(f, "{}", bold.suffix())?;
+    write!(f, "{}", message.suffix())?;
 
     if let Some(token) = error_token {
       write_token_error_context(f, token)?;


Test patch:
diff --git a/src/test_utils.rs b/src/test_utils.rs
index 5672a115..9efabaf0 100644
--- a/src/test_utils.rs
+++ b/src/test_utils.rs
@@ -3,22 +3,12 @@ extern crate glob;
 use ::prelude::*;
 
 pub fn just_binary_path() -> PathBuf {
-  let exe = String::from("just") + env::consts::EXE_SUFFIX;
-
-  let mut path = env::current_dir().unwrap();
-  path.push("target");
-  path.push("debug");
-  path.push(&exe);
-
-  if !path.is_file() {
-    let mut pattern = env::current_dir().unwrap();
-    pattern.push("target");
-    pattern.push("*");
-    pattern.push("debug");
-    pattern.push(&exe);
-    path = glob::glob(pattern.to_str().unwrap()).unwrap()
-      .take_while(Result::is_ok).nth(0).unwrap().unwrap();
+  let mut path = env::current_exe().unwrap();
+  path.pop();
+  if path.ends_with("deps") {
+    path.pop();
   }
-
+  let exe = String::from("just") + env::consts::EXE_SUFFIX;
+  path.push(exe);
   path
 }