From fa520239060aba86ccd33d7588269b84e036e35b Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 15 May 2023 14:23:10 -0500 Subject: [PATCH] Add some color to error messages (#1028) * Add some color to error messages This commit updates the errors printed by the CLI to have `error` in red, the main error message in bold, and the rest of it is unchanged. * Fix tests * Fix no-default-features build --- .../compositions/component-missing/error.txt | 2 +- .../missing-explicit-dep/error.txt | 2 +- .../tests/compositions/missing-root/error.txt | 2 +- crates/wat/src/lib.rs | 4 +- src/bin/wasm-tools/addr2line.rs | 4 ++ src/bin/wasm-tools/component.rs | 23 +++++++- src/bin/wasm-tools/compose.rs | 7 ++- src/bin/wasm-tools/demangle.rs | 4 ++ src/bin/wasm-tools/dump.rs | 4 ++ src/bin/wasm-tools/main.rs | 56 ++++++++++++++++++- src/bin/wasm-tools/metadata.rs | 15 +++++ src/bin/wasm-tools/mutate.rs | 4 ++ src/bin/wasm-tools/objdump.rs | 6 +- src/bin/wasm-tools/parse.rs | 4 ++ src/bin/wasm-tools/print.rs | 4 ++ src/bin/wasm-tools/shrink.rs | 4 ++ src/bin/wasm-tools/smith.rs | 7 ++- src/bin/wasm-tools/strip.rs | 4 ++ src/bin/wasm-tools/validate.rs | 4 ++ src/bin/wasm-tools/wit_smith.rs | 7 ++- src/lib.rs | 24 ++++---- 21 files changed, 160 insertions(+), 31 deletions(-) diff --git a/crates/wasm-compose/tests/compositions/component-missing/error.txt b/crates/wasm-compose/tests/compositions/component-missing/error.txt index 61c880b6a6..dde33c0936 100644 --- a/crates/wasm-compose/tests/compositions/component-missing/error.txt +++ b/crates/wasm-compose/tests/compositions/component-missing/error.txt @@ -1,5 +1,5 @@ failed to parse component `tests/compositions/component-missing/a.wat` Caused by: - 0: failed to read from `tests/compositions/component-missing/a.wat`: No such file or directory (os error 2) + 0: failed to read from `tests/compositions/component-missing/a.wat` 1: No such file or directory (os error 2) diff --git a/crates/wasm-compose/tests/compositions/missing-explicit-dep/error.txt b/crates/wasm-compose/tests/compositions/missing-explicit-dep/error.txt index 36521b6996..415ffcae6f 100644 --- a/crates/wasm-compose/tests/compositions/missing-explicit-dep/error.txt +++ b/crates/wasm-compose/tests/compositions/missing-explicit-dep/error.txt @@ -1,5 +1,5 @@ failed to parse component `tests/compositions/missing-explicit-dep/a.wat` Caused by: - 0: failed to read from `tests/compositions/missing-explicit-dep/a.wat`: No such file or directory (os error 2) + 0: failed to read from `tests/compositions/missing-explicit-dep/a.wat` 1: No such file or directory (os error 2) diff --git a/crates/wasm-compose/tests/compositions/missing-root/error.txt b/crates/wasm-compose/tests/compositions/missing-root/error.txt index 8778fc129c..82aefdb9c3 100644 --- a/crates/wasm-compose/tests/compositions/missing-root/error.txt +++ b/crates/wasm-compose/tests/compositions/missing-root/error.txt @@ -1,5 +1,5 @@ failed to parse component `tests/compositions/missing-root/root.wat` Caused by: - 0: failed to read from `tests/compositions/missing-root/root.wat`: No such file or directory (os error 2) + 0: failed to read from `tests/compositions/missing-root/root.wat` 1: No such file or directory (os error 2) diff --git a/crates/wat/src/lib.rs b/crates/wat/src/lib.rs index bd0bafcb71..c16174602e 100644 --- a/crates/wat/src/lib.rs +++ b/crates/wat/src/lib.rs @@ -287,7 +287,7 @@ impl fmt::Display for Error { }, ErrorKind::Io { err, file, .. } => match file { Some(file) => { - write!(f, "failed to read from `{}`: {}", file.display(), err) + write!(f, "failed to read from `{}`", file.display()) } None => err.fmt(f), }, @@ -321,7 +321,7 @@ mod test { let e = parse_file("_does_not_exist_").unwrap_err(); assert!(e .to_string() - .starts_with("failed to read from `_does_not_exist_`: ")); + .starts_with("failed to read from `_does_not_exist_`")); let mut e = parse_bytes("()".as_bytes()).unwrap_err(); e.set_path("foo"); diff --git a/src/bin/wasm-tools/addr2line.rs b/src/bin/wasm-tools/addr2line.rs index c6f8cf8516..5478a7cb45 100644 --- a/src/bin/wasm-tools/addr2line.rs +++ b/src/bin/wasm-tools/addr2line.rs @@ -41,6 +41,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let wasm = self.io.parse_input_wasm()?; diff --git a/src/bin/wasm-tools/component.rs b/src/bin/wasm-tools/component.rs index 1ec1cbd45c..544ec5d621 100644 --- a/src/bin/wasm-tools/component.rs +++ b/src/bin/wasm-tools/component.rs @@ -26,6 +26,14 @@ impl Opts { Opts::Embed(embed) => embed.run(), } } + + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + match self { + Opts::New(new) => new.general_opts(), + Opts::Wit(wit) => wit.general_opts(), + Opts::Embed(embed) => embed.general_opts(), + } + } } fn parse_optionally_name_file(s: &str) -> (&str, &str) { @@ -91,6 +99,10 @@ pub struct NewOpts { } impl NewOpts { + fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + /// Executes the application. fn run(self) -> Result<()> { let wasm = self.io.parse_input_wasm()?; @@ -172,10 +184,13 @@ pub struct EmbedOpts { } impl EmbedOpts { + fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + /// Executes the application. fn run(self) -> Result<()> { let wasm = if self.dummy { - self.io.init_logger(); None } else { Some(self.io.parse_input_wasm()?) @@ -217,7 +232,7 @@ impl EmbedOpts { #[derive(Parser)] pub struct WitOpts { #[clap(flatten)] - verbosity: wasm_tools::Verbosity, + general: wasm_tools::GeneralOpts, /// Input file or directory to process. /// @@ -275,6 +290,10 @@ pub struct WitOpts { } impl WitOpts { + fn general_opts(&self) -> &wasm_tools::GeneralOpts { + &self.general + } + /// Executes the application. fn run(self) -> Result<()> { let name = match &self.name { diff --git a/src/bin/wasm-tools/compose.rs b/src/bin/wasm-tools/compose.rs index 3ebe6ce3eb..c923650916 100644 --- a/src/bin/wasm-tools/compose.rs +++ b/src/bin/wasm-tools/compose.rs @@ -7,12 +7,15 @@ pub struct Opts { #[clap(flatten)] cmd: WasmComposeCommand, #[clap(flatten)] - verbosity: wasm_tools::Verbosity, + general: wasm_tools::GeneralOpts, } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + &self.general + } + pub fn run(self) -> Result<()> { - self.verbosity.init_logger(); self.cmd.execute() } } diff --git a/src/bin/wasm-tools/demangle.rs b/src/bin/wasm-tools/demangle.rs index be3b51a8b7..262ec3122c 100644 --- a/src/bin/wasm-tools/demangle.rs +++ b/src/bin/wasm-tools/demangle.rs @@ -19,6 +19,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let input = self.io.parse_input_wasm()?; let mut module = wasm_encoder::Module::new(); diff --git a/src/bin/wasm-tools/dump.rs b/src/bin/wasm-tools/dump.rs index 994dc92ec8..26b572b539 100644 --- a/src/bin/wasm-tools/dump.rs +++ b/src/bin/wasm-tools/dump.rs @@ -15,6 +15,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let input = self.io.parse_input_wasm()?; let output = self.io.output_writer()?; diff --git a/src/bin/wasm-tools/main.rs b/src/bin/wasm-tools/main.rs index 3f16927bd1..e176f6713d 100644 --- a/src/bin/wasm-tools/main.rs +++ b/src/bin/wasm-tools/main.rs @@ -1,7 +1,8 @@ use anyhow::Result; use clap::Parser; -use std::io; +use std::io::{self, Write}; use std::process::ExitCode; +use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor}; macro_rules! subcommands { ($( @@ -33,6 +34,15 @@ macro_rules! subcommands { )* } } + + fn general_opts(&self) -> &wasm_tools::GeneralOpts { + match *self { + $( + #[cfg(feature = $string)] + Self::$name(ref opts) => opts.general_opts(), + )* + } + } } } } @@ -58,7 +68,10 @@ subcommands! { } fn main() -> ExitCode { - let err = match ::parse().run() { + let args = ::parse(); + args.general_opts().init_logger(); + let color = args.general_opts().color; + let err = match args.run() { Ok(()) => return ExitCode::SUCCESS, Err(e) => e, }; @@ -72,10 +85,47 @@ fn main() -> ExitCode { _ => {} } } - eprintln!("Error: {:?}", err); + + // ignore errors here since if we fail to print an error it's not like we + // can print it again. + let _ = print_error(color, err); ExitCode::FAILURE } +fn print_error(color: ColorChoice, err: anyhow::Error) -> Result<()> { + let color = if color == ColorChoice::Auto && !atty::is(atty::Stream::Stderr) { + ColorChoice::Never + } else { + color + }; + let mut stderr = StandardStream::stderr(color); + stderr.set_color(ColorSpec::new().set_fg(Some(Color::Red)).set_bold(true))?; + write!(stderr, "error")?; + stderr.set_color(ColorSpec::new().set_fg(None).set_bold(true))?; + write!(stderr, ": ")?; + + let msg = err.to_string(); + for (i, line) in msg.lines().enumerate() { + writeln!(stderr, "{line}")?; + if i == 0 { + stderr.set_color(ColorSpec::new().set_reset(true))?; + } + } + + if err.chain().len() == 1 { + return Ok(()); + } + writeln!(stderr, "\nCaused by:")?; + for (i, err) in err.chain().skip(1).enumerate() { + writeln!( + stderr, + "{i:>5}: {}", + err.to_string().replace("\n", "\n ") + )?; + } + return Ok(()); +} + /// If CARGO_VERSION_INFO is set, use it, otherwise use CARGO_PKG_VERSION. fn version() -> &'static str { option_env!("CARGO_VERSION_INFO").unwrap_or(env!("CARGO_PKG_VERSION")) diff --git a/src/bin/wasm-tools/metadata.rs b/src/bin/wasm-tools/metadata.rs index 041918a061..83142db550 100644 --- a/src/bin/wasm-tools/metadata.rs +++ b/src/bin/wasm-tools/metadata.rs @@ -15,6 +15,13 @@ impl Opts { Opts::Add(opts) => opts.run(), } } + + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + match self { + Opts::Show(opts) => opts.general_opts(), + Opts::Add(opts) => opts.general_opts(), + } + } } /// Read metadata (module name, producers) from a WebAssembly file. @@ -29,6 +36,10 @@ pub struct ShowOpts { } impl ShowOpts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let input = self.io.parse_input_wasm()?; let mut output = self.io.output_writer()?; @@ -58,6 +69,10 @@ pub struct AddOpts { } impl AddOpts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let input = self.io.parse_input_wasm()?; diff --git a/src/bin/wasm-tools/mutate.rs b/src/bin/wasm-tools/mutate.rs index 2dd0f60eef..acb5a7e413 100644 --- a/src/bin/wasm-tools/mutate.rs +++ b/src/bin/wasm-tools/mutate.rs @@ -49,6 +49,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(mut self) -> Result<()> { let input_wasm = self.io.parse_input_wasm()?; diff --git a/src/bin/wasm-tools/objdump.rs b/src/bin/wasm-tools/objdump.rs index 939c4bf012..248fc18396 100644 --- a/src/bin/wasm-tools/objdump.rs +++ b/src/bin/wasm-tools/objdump.rs @@ -1,7 +1,7 @@ use anyhow::Result; -use termcolor::WriteColor; use std::io::Write; use std::ops::Range; +use termcolor::WriteColor; use wasmparser::{Encoding, Parser, Payload::*}; /// Dumps information about sections in a WebAssembly file. @@ -15,6 +15,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let input = self.io.parse_input_wasm()?; diff --git a/src/bin/wasm-tools/parse.rs b/src/bin/wasm-tools/parse.rs index 4f83358a50..3274bfbeca 100644 --- a/src/bin/wasm-tools/parse.rs +++ b/src/bin/wasm-tools/parse.rs @@ -16,6 +16,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let binary = self.io.parse_input_wasm()?; self.io.output(wasm_tools::Output::Wasm { diff --git a/src/bin/wasm-tools/print.rs b/src/bin/wasm-tools/print.rs index d76c030fc2..a7eae6f572 100644 --- a/src/bin/wasm-tools/print.rs +++ b/src/bin/wasm-tools/print.rs @@ -20,6 +20,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let wasm = self.io.parse_input_wasm()?; let mut printer = wasmprinter::Printer::new(); diff --git a/src/bin/wasm-tools/shrink.rs b/src/bin/wasm-tools/shrink.rs index b13c0f8f9a..c70f550d53 100644 --- a/src/bin/wasm-tools/shrink.rs +++ b/src/bin/wasm-tools/shrink.rs @@ -36,6 +36,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(self) -> Result<()> { let input = self.io.parse_input_wasm()?; let initial_size = input.len(); diff --git a/src/bin/wasm-tools/smith.rs b/src/bin/wasm-tools/smith.rs index a4bcc02ef9..fac3546e12 100644 --- a/src/bin/wasm-tools/smith.rs +++ b/src/bin/wasm-tools/smith.rs @@ -76,7 +76,7 @@ pub struct Opts { module_config: Config, #[clap(flatten)] - verbosity: wasm_tools::Verbosity, + general: wasm_tools::GeneralOpts, } #[derive(Default, Debug, Parser, Clone, serde::Deserialize)] @@ -195,8 +195,11 @@ struct Config { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + &self.general + } + pub fn run(&self) -> Result<()> { - self.verbosity.init_logger(); let seed = match &self.input { Some(f) => { std::fs::read(f).with_context(|| format!("failed to read '{}'", f.display()))? diff --git a/src/bin/wasm-tools/strip.rs b/src/bin/wasm-tools/strip.rs index f68c678237..5b66afa235 100644 --- a/src/bin/wasm-tools/strip.rs +++ b/src/bin/wasm-tools/strip.rs @@ -27,6 +27,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { let input = self.io.parse_input_wasm()?; let to_delete = regex::RegexSet::new(self.delete.iter())?; diff --git a/src/bin/wasm-tools/validate.rs b/src/bin/wasm-tools/validate.rs index e4ee61a3e2..76476f899e 100644 --- a/src/bin/wasm-tools/validate.rs +++ b/src/bin/wasm-tools/validate.rs @@ -38,6 +38,10 @@ pub struct Opts { } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + self.io.general_opts() + } + pub fn run(&self) -> Result<()> { // Note that here we're copying the contents of // `Validator::validate_all`, but the end is followed up with a parallel diff --git a/src/bin/wasm-tools/wit_smith.rs b/src/bin/wasm-tools/wit_smith.rs index ad370dfe8f..19716e6c64 100644 --- a/src/bin/wasm-tools/wit_smith.rs +++ b/src/bin/wasm-tools/wit_smith.rs @@ -42,12 +42,15 @@ pub struct Opts { arbitrary_config: bool, #[clap(flatten)] - verbosity: wasm_tools::Verbosity, + general: wasm_tools::GeneralOpts, } impl Opts { + pub fn general_opts(&self) -> &wasm_tools::GeneralOpts { + &self.general + } + pub fn run(&self) -> Result<()> { - self.verbosity.init_logger(); let seed = match &self.input { Some(f) => { std::fs::read(f).with_context(|| format!("failed to read '{}'", f.display()))? diff --git a/src/lib.rs b/src/lib.rs index e94832f97d..a5693aceec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,15 +6,18 @@ use std::io::{BufWriter, Read, Write}; use std::path::{Path, PathBuf}; use termcolor::{Ansi, ColorChoice, NoColor, StandardStream, WriteColor}; -// Implements the verbosity flag for the CLI commands. #[derive(clap::Parser)] -pub struct Verbosity { +pub struct GeneralOpts { /// Use verbose output (-vv very verbose output). #[clap(long = "verbose", short = 'v', action = clap::ArgAction::Count)] verbose: u8, + + /// Use colors in output. + #[clap(long = "color", default_value = "auto")] + pub color: ColorChoice, } -impl Verbosity { +impl GeneralOpts { /// Initializes the logger based on the verbosity level. pub fn init_logger(&self) { let default = match self.verbose { @@ -48,11 +51,7 @@ pub struct InputOutput { output: OutputArg, #[clap(flatten)] - verbosity: Verbosity, - - /// Use colors in output. - #[clap(long = "color", short = 'c')] - color: Option, + general: GeneralOpts, } #[derive(clap::Parser)] @@ -71,8 +70,6 @@ pub enum Output<'a> { impl InputOutput { pub fn parse_input_wasm(&self) -> Result> { - self.verbosity.init_logger(); - if let Some(path) = &self.input { if path != Path::new("-") { let bytes = wat::parse_file(path)?; @@ -95,8 +92,7 @@ impl InputOutput { } pub fn output_writer(&self) -> Result> { - self.output - .output_writer(self.color.unwrap_or(ColorChoice::Auto)) + self.output.output_writer(self.general.color) } pub fn output_path(&self) -> Option<&Path> { @@ -107,8 +103,8 @@ impl InputOutput { self.input.as_deref() } - pub fn init_logger(&self) { - self.verbosity.init_logger(); + pub fn general_opts(&self) -> &GeneralOpts { + &self.general } }