From 27cf2b96df127dd9475a5b1c1c412a13124edbad Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Wed, 28 Jul 2021 18:06:57 -0700 Subject: [PATCH] Use ColorDisplay trait to print objects to the terminal (#926) --- src/color.rs | 8 -------- src/color_display.rs | 20 ++++++++++++++++++ src/common.rs | 4 ++-- src/error.rs | 44 ++++++++++++++++------------------------ src/interrupt_handler.rs | 21 +++++++++++++------ src/item.rs | 2 +- src/justfile.rs | 10 ++++----- src/lexer.rs | 12 ++++------- src/lib.rs | 1 + src/load_dotenv.rs | 7 ++++--- src/parameter.rs | 5 ++--- src/recipe.rs | 6 +++--- src/run.rs | 2 +- src/subcommand.rs | 34 +++++++++++++++---------------- src/token.rs | 14 +++++++------ src/warning.rs | 15 +++++++------- 16 files changed, 107 insertions(+), 98 deletions(-) create mode 100644 src/color_display.rs diff --git a/src/color.rs b/src/color.rs index 0e21580..18340c3 100644 --- a/src/color.rs +++ b/src/color.rs @@ -30,14 +30,6 @@ impl Color { } } - pub(crate) fn fmt(fmt: &Formatter) -> Self { - if fmt.alternate() { - Self::always() - } else { - Self::never() - } - } - pub(crate) fn auto() -> Self { Self { use_color: UseColor::Auto, diff --git a/src/color_display.rs b/src/color_display.rs new file mode 100644 index 0000000..b9eab60 --- /dev/null +++ b/src/color_display.rs @@ -0,0 +1,20 @@ +use crate::common::*; + +pub(crate) trait ColorDisplay { + fn color_display<'a>(&'a self, color: Color) -> Wrapper<'a> + where + Self: Sized, + { + Wrapper(self, color) + } + + fn fmt(&self, f: &mut Formatter, color: Color) -> fmt::Result; +} + +pub(crate) struct Wrapper<'a>(&'a dyn ColorDisplay, Color); + +impl<'a> Display for Wrapper<'a> { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + self.0.fmt(f, self.1) + } +} diff --git a/src/common.rs b/src/common.rs index 5759423..12e558d 100644 --- a/src/common.rs +++ b/src/common.rs @@ -38,8 +38,8 @@ pub(crate) use crate::{load_dotenv::load_dotenv, output::output, unindent::unind // traits pub(crate) use crate::{ - command_ext::CommandExt, keyed::Keyed, ordinal::Ordinal, platform_interface::PlatformInterface, - range_ext::RangeExt, + color_display::ColorDisplay, command_ext::CommandExt, keyed::Keyed, ordinal::Ordinal, + platform_interface::PlatformInterface, range_ext::RangeExt, }; // structs and enums diff --git a/src/error.rs b/src/error.rs index b9437a0..377b190 100644 --- a/src/error.rs +++ b/src/error.rs @@ -160,30 +160,6 @@ impl<'src> Error<'src> { message: message.into(), } } - - pub(crate) fn write(&self, w: &mut dyn Write, color: Color) -> io::Result<()> { - let color = color.stderr(); - - if color.active() { - writeln!( - w, - "{}: {}{:#}{}", - color.error().paint("error"), - color.message().prefix(), - self, - color.message().suffix() - )?; - } else { - writeln!(w, "error: {}", self)?; - } - - if let Some(token) = self.context() { - token.write_context(w, color.error())?; - writeln!(w)?; - } - - Ok(()) - } } impl<'src> From> for Error<'src> { @@ -210,10 +186,17 @@ impl<'src> From for Error<'src> { } } -impl<'src> Display for Error<'src> { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { +impl<'src> ColorDisplay for Error<'src> { + fn fmt(&self, f: &mut Formatter, color: Color) -> fmt::Result { use Error::*; + write!( + f, + "{}: {}", + color.error().paint("error"), + color.message().prefix() + )?; + match self { ArgumentCountMismatch { recipe, @@ -254,7 +237,7 @@ impl<'src> Display for Error<'src> { } write!(f, "\nusage:\n just {}", recipe)?; for param in parameters { - write!(f, " {}", param)?; + write!(f, " {}", param.color_display(color))?; } }, Backtick { output_error, .. } => match output_error { @@ -619,6 +602,13 @@ impl<'src> Display for Error<'src> { }, } + write!(f, "{}", color.message().suffix())?; + + if let Some(token) = self.context() { + writeln!(f)?; + write!(f, "{}", token.color_display(color.error()))?; + } + Ok(()) } } diff --git a/src/interrupt_handler.rs b/src/interrupt_handler.rs index a3f2dc7..acba2dd 100644 --- a/src/interrupt_handler.rs +++ b/src/interrupt_handler.rs @@ -21,9 +21,13 @@ impl InterruptHandler { match INSTANCE.lock() { Ok(guard) => guard, Err(poison_error) => { - eprintln!("{}", Error::Internal { - message: format!("interrupt handler mutex poisoned: {}", poison_error), - }); + eprintln!( + "{}", + Error::Internal { + message: format!("interrupt handler mutex poisoned: {}", poison_error), + } + .color_display(Color::auto().stderr()) + ); std::process::exit(EXIT_FAILURE); }, } @@ -58,9 +62,14 @@ impl InterruptHandler { pub(crate) fn unblock(&mut self) { if self.blocks == 0 { if self.verbosity.loud() { - eprintln!("{}", Error::Internal { - message: "attempted to unblock interrupt handler, but handler was not blocked".to_owned(), - }); + eprintln!( + "{}", + Error::Internal { + message: "attempted to unblock interrupt handler, but handler was not blocked" + .to_owned(), + } + .color_display(Color::auto().stderr()) + ); } std::process::exit(EXIT_FAILURE); } diff --git a/src/item.rs b/src/item.rs index 21fa54a..2e527c7 100644 --- a/src/item.rs +++ b/src/item.rs @@ -16,7 +16,7 @@ impl<'src> Display for Item<'src> { Item::Alias(alias) => write!(f, "{}", alias), Item::Assignment(assignment) => write!(f, "{}", assignment), Item::Comment(comment) => write!(f, "{}", comment), - Item::Recipe(recipe) => write!(f, "{}", recipe), + Item::Recipe(recipe) => write!(f, "{}", recipe.color_display(Color::never())), Item::Set(set) => write!(f, "{}", set), } } diff --git a/src/justfile.rs b/src/justfile.rs index 7d5c762..151c4fe 100644 --- a/src/justfile.rs +++ b/src/justfile.rs @@ -375,8 +375,8 @@ impl<'src> Justfile<'src> { } } -impl<'src> Display for Justfile<'src> { - fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { +impl<'src> ColorDisplay for Justfile<'src> { + fn fmt(&self, f: &mut Formatter, color: Color) -> Result<(), fmt::Error> { let mut items = self.recipes.len() + self.assignments.len() + self.aliases.len(); for (name, assignment) in &self.assignments { if assignment.export { @@ -396,7 +396,7 @@ impl<'src> Display for Justfile<'src> { } } for recipe in self.recipes.values() { - write!(f, "{}", recipe)?; + write!(f, "{}", recipe.color_display(color))?; items -= 1; if items != 0 { write!(f, "\n\n")?; @@ -683,11 +683,11 @@ mod tests { fn test(input: &str, expected: &str) { let justfile = compile(input); - let actual = format!("{:#}", justfile); + let actual = format!("{}", justfile.color_display(Color::never())); assert_eq!(actual, expected); println!("Re-parsing..."); let reparsed = compile(&actual); - let redumped = format!("{:#}", reparsed); + let redumped = format!("{}", reparsed.color_display(Color::never())); assert_eq!(redumped, actual); } diff --git a/src/lexer.rs b/src/lexer.rs index 6e57cef..f68cf65 100644 --- a/src/lexer.rs +++ b/src/lexer.rs @@ -2298,17 +2298,13 @@ mod tests { } if message == "Lexer presumed character `-`" ); - let mut cursor = Cursor::new(Vec::new()); - - Error::Compile { compile_error } - .write(&mut cursor, Color::never()) - .unwrap(); - assert_eq!( - str::from_utf8(&cursor.into_inner()).unwrap(), + Error::Compile { compile_error } + .color_display(Color::never()) + .to_string(), "error: Internal error, this may indicate a bug in just: \ Lexer presumed character `-`\nconsider filing an issue: \ - https://github.com/casey/just/issues/new\n |\n1 | !\n | ^\n" + https://github.com/casey/just/issues/new\n |\n1 | !\n | ^" ); } } diff --git a/src/lib.rs b/src/lib.rs index c7f125d..448146c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -65,6 +65,7 @@ mod assignment_resolver; mod ast; mod binding; mod color; +mod color_display; mod command_ext; mod common; mod compile_error; diff --git a/src/load_dotenv.rs b/src/load_dotenv.rs index babda29..a91fbc2 100644 --- a/src/load_dotenv.rs +++ b/src/load_dotenv.rs @@ -23,9 +23,10 @@ pub(crate) fn load_dotenv( .map(|val| val.as_os_str().to_str() == Some("1")) .unwrap_or(false) { - Warning::DotenvLoad - .write(&mut io::stderr(), config.color.stderr()) - .ok(); + eprintln!( + "{}", + Warning::DotenvLoad.color_display(config.color.stderr()) + ); } let iter = dotenv::from_path_iter(&path)?; diff --git a/src/parameter.rs b/src/parameter.rs index ee0d93b..521392d 100644 --- a/src/parameter.rs +++ b/src/parameter.rs @@ -13,9 +13,8 @@ pub(crate) struct Parameter<'src> { pub(crate) export: bool, } -impl<'src> Display for Parameter<'src> { - fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { - let color = Color::fmt(f); +impl<'src> ColorDisplay for Parameter<'src> { + fn fmt(&self, f: &mut Formatter, color: Color) -> Result<(), fmt::Error> { if self.export { write!(f, "$")?; } diff --git a/src/recipe.rs b/src/recipe.rs index 184f711..72e4886 100644 --- a/src/recipe.rs +++ b/src/recipe.rs @@ -309,8 +309,8 @@ impl<'src, D> Keyed<'src> for Recipe<'src, D> { } } -impl<'src, D: Display> Display for Recipe<'src, D> { - fn fmt(&self, f: &mut Formatter) -> Result<(), fmt::Error> { +impl<'src, D: Display> ColorDisplay for Recipe<'src, D> { + fn fmt(&self, f: &mut Formatter, color: Color) -> Result<(), fmt::Error> { if let Some(doc) = self.doc { writeln!(f, "# {}", doc)?; } @@ -322,7 +322,7 @@ impl<'src, D: Display> Display for Recipe<'src, D> { } for parameter in &self.parameters { - write!(f, " {}", parameter)?; + write!(f, " {}", parameter.color_display(color))?; } write!(f, ":")?; diff --git a/src/run.rs b/src/run.rs index e7f3568..4bf8c04 100644 --- a/src/run.rs +++ b/src/run.rs @@ -30,7 +30,7 @@ pub fn run() -> Result<(), i32> { }) .map_err(|error| { if !verbosity.quiet() { - error.write(&mut io::stderr(), color).ok(); + eprintln!("{}", error.color_display(color)); } error.code().unwrap_or(EXIT_FAILURE) }) diff --git a/src/subcommand.rs b/src/subcommand.rs index bbc8462..b7a072d 100644 --- a/src/subcommand.rs +++ b/src/subcommand.rs @@ -62,24 +62,24 @@ impl Subcommand { if config.verbosity.loud() { for warning in &justfile.warnings { - warning.write(&mut io::stderr(), config.color.stderr()).ok(); + eprintln!("{}", warning.color_display(config.color.stderr())); } } match self { Choose { overrides, chooser } => - Self::choose(&config, justfile, &search, overrides, chooser.as_deref())?, - Command { overrides, .. } => justfile.run(&config, &search, overrides, &[])?, + Self::choose(config, justfile, &search, overrides, chooser.as_deref())?, + Command { overrides, .. } => justfile.run(config, &search, overrides, &[])?, Dump => Self::dump(ast), - Evaluate { overrides, .. } => justfile.run(&config, &search, overrides, &[])?, - Format => Self::format(&config, ast, &search)?, - List => Self::list(&config, justfile), + Evaluate { overrides, .. } => justfile.run(config, &search, overrides, &[])?, + Format => Self::format(config, ast, &search)?, + List => Self::list(config, justfile), Run { arguments, overrides, - } => justfile.run(&config, &search, overrides, arguments)?, - Show { ref name } => Self::show(&name, justfile)?, - Summary => Self::summary(&config, justfile), + } => justfile.run(config, &search, overrides, arguments)?, + Show { ref name } => Self::show(config, &name, justfile)?, + Summary => Self::summary(config, justfile), Variables => Self::variables(justfile), Completions { .. } | Edit | Init => unreachable!(), } @@ -308,7 +308,9 @@ impl Subcommand { let mut line_width = UnicodeWidthStr::width(*name); for parameter in &recipe.parameters { - line_width += UnicodeWidthStr::width(format!(" {}", parameter).as_str()); + line_width += UnicodeWidthStr::width( + format!(" {}", parameter.color_display(Color::never())).as_str(), + ); } if line_width <= 30 { @@ -331,11 +333,7 @@ impl Subcommand { { print!("{}{}", config.list_prefix, name); for parameter in &recipe.parameters { - if config.color.stdout().active() { - print!(" {:#}", parameter); - } else { - print!(" {}", parameter); - } + print!(" {}", parameter.color_display(config.color.stdout())); } // Declaring this outside of the nested loops will probably be more efficient, @@ -365,14 +363,14 @@ impl Subcommand { } } - fn show<'src>(name: &str, justfile: Justfile<'src>) -> Result<(), Error<'src>> { + fn show<'src>(config: &Config, name: &str, justfile: Justfile<'src>) -> Result<(), Error<'src>> { if let Some(alias) = justfile.get_alias(name) { let recipe = justfile.get_recipe(alias.target.name.lexeme()).unwrap(); println!("{}", alias); - println!("{}", recipe); + println!("{}", recipe.color_display(config.color.stdout())); Ok(()) } else if let Some(recipe) = justfile.get_recipe(name) { - println!("{}", recipe); + println!("{}", recipe.color_display(config.color.stdout())); Ok(()) } else { Err(Error::UnknownRecipes { diff --git a/src/token.rs b/src/token.rs index 8905db8..15d0b07 100644 --- a/src/token.rs +++ b/src/token.rs @@ -18,8 +18,10 @@ impl<'src> Token<'src> { pub(crate) fn error(&self, kind: CompileErrorKind<'src>) -> CompileError<'src> { CompileError { token: *self, kind } } +} - pub(crate) fn write_context(&self, w: &mut dyn Write, color: Color) -> io::Result<()> { +impl<'src> ColorDisplay for Token<'src> { + fn fmt(&self, f: &mut Formatter, color: Color) -> fmt::Result { let width = if self.length == 0 { 1 } else { self.length }; let line_number = self.line.ordinal(); @@ -50,11 +52,11 @@ impl<'src> Token<'src> { i += c.len_utf8(); } let line_number_width = line_number.to_string().len(); - writeln!(w, "{0:1$} |", "", line_number_width)?; - writeln!(w, "{} | {}", line_number, space_line)?; - write!(w, "{0:1$} |", "", line_number_width)?; + writeln!(f, "{0:1$} |", "", line_number_width)?; + writeln!(f, "{} | {}", line_number, space_line)?; + write!(f, "{0:1$} |", "", line_number_width)?; write!( - w, + f, " {0:1$}{2}{3:^<4$}{5}", "", space_column, @@ -67,7 +69,7 @@ impl<'src> Token<'src> { None => if self.offset != self.src.len() { write!( - w, + f, "internal error: Error has invalid line number: {}", line_number )?; diff --git a/src/warning.rs b/src/warning.rs index b37e8a7..d55860c 100644 --- a/src/warning.rs +++ b/src/warning.rs @@ -2,8 +2,6 @@ use crate::common::*; #[derive(Clone, Debug, PartialEq)] pub(crate) enum Warning { - // Remove this on 2021-07-01. - #[allow(dead_code)] DotenvLoad, } @@ -13,17 +11,19 @@ impl Warning { Self::DotenvLoad => None, } } +} - pub(crate) fn write(&self, w: &mut dyn Write, color: Color) -> io::Result<()> { +impl ColorDisplay for Warning { + fn fmt(&self, f: &mut Formatter, color: Color) -> fmt::Result { let warning = color.warning(); let message = color.message(); - write!(w, "{} {}", warning.paint("warning:"), message.prefix())?; + write!(f, "{} {}", warning.paint("warning:"), message.prefix())?; match self { Self::DotenvLoad => { #[rustfmt::skip] - write!(w, "\ + write!(f, "\ A `.env` file was found and loaded, but this behavior will change in the future. To silence this warning and continue loading `.env` files, add: @@ -44,10 +44,11 @@ See https://github.com/casey/just/issues/469 for more details.")?; }, } - writeln!(w, "{}", message.suffix())?; + write!(f, "{}", message.suffix())?; if let Some(token) = self.context() { - token.write_context(w, warning)?; + writeln!(f)?; + write!(f, "{}", token.color_display(color))?; } Ok(())