From 1b1a155ddae2e4bfc2bbc62cabf1b42780b05e7e Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Thu, 1 Jun 2017 18:01:35 -0700 Subject: [PATCH] Refactor color handling (#204) 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. --- src/app.rs | 74 ++++------------------- src/color.rs | 145 +++++++++++++++++++++++++++++++++++++++++++++ src/integration.rs | 26 ++++++++ src/lib.rs | 106 ++++++++++----------------------- src/test_utils.rs | 22 ++----- 5 files changed, 222 insertions(+), 151 deletions(-) create mode 100644 src/color.rs diff --git a/src/app.rs b/src/app.rs index 4914fa8..867f5f2 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 { - 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>(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 0000000..9b89144 --- /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 c13d4d9..5c9d2c8 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 ea4c361..da585c9 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::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::();; - 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, ) -> 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)?; diff --git a/src/test_utils.rs b/src/test_utils.rs index 5672a11..9efabaf 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 }