From 74c46d60907a591ed3636aae3d6515a75c949a66 Mon Sep 17 00:00:00 2001 From: Jane Lusby Date: Sun, 26 Jul 2020 14:16:59 -0700 Subject: [PATCH] fix output format to be more consistent (#43) * fix output format to be more consistent * cleanup header writer interface --- src/config.rs | 222 ++++++++++++++++++++++++++------------------- src/handler.rs | 45 +++++---- src/section/mod.rs | 18 ++-- src/writers.rs | 142 ++++++++++++++++++++++++++--- 4 files changed, 292 insertions(+), 135 deletions(-) diff --git a/src/config.rs b/src/config.rs index a326c0b..a88891a 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,8 +1,13 @@ //! Configuration options for customizing the behavior of the provided panic //! and error reporting hooks -use crate::ColorExt; +use crate::{ + writers::{EnvSection, WriterExt}, + ColorExt, +}; use ansi_term::Color::*; +use indenter::{indented, Format}; use std::env; +use std::fmt::Write as _; use std::{fmt, path::PathBuf, sync::Arc}; #[derive(Debug)] @@ -54,25 +59,27 @@ impl fmt::Display for Frame { }; write!(f, "{}", color.paint(&name[..name.len() - 19]))?; let color = Black.make_intense(); - writeln!(f, "{}", color.paint(&name[name.len() - 19..]))?; + write!(f, "{}", color.paint(&name[name.len() - 19..]))?; } else { - writeln!(f, "{}", name)?; + write!(f, "{}", name)?; } + let mut separated = f.header("\n"); + // Print source location, if known. if let Some(ref file) = self.filename { let filestr = file.to_str().unwrap_or(""); let lineno = self .lineno .map_or("".to_owned(), |x| x.to_string()); - writeln!( - f, + write!( + &mut separated.ready(), " at {}:{}", Purple.paint(filestr), Purple.paint(lineno) )?; } else { - writeln!(f, " at ")?; + write!(&mut separated.ready(), " at ")?; } let v = if std::thread::panicking() { @@ -83,7 +90,50 @@ impl fmt::Display for Frame { // Maybe print source. if v >= Verbosity::Full { - self.print_source_if_avail(f)?; + write!(&mut separated.ready(), "{}", SourceSection(self))?; + } + + Ok(()) + } +} + +struct SourceSection<'a>(&'a Frame); + +impl fmt::Display for SourceSection<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let (lineno, filename) = match (self.0.lineno, self.0.filename.as_ref()) { + (Some(a), Some(b)) => (a, b), + // Without a line number and file name, we can't sensibly proceed. + _ => return Ok(()), + }; + + let file = match std::fs::File::open(filename) { + Ok(file) => file, + Err(ref e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), + e @ Err(_) => e.unwrap(), + }; + + use std::fmt::Write; + use std::io::BufRead; + + // Extract relevant lines. + let reader = std::io::BufReader::new(file); + let start_line = lineno - 2.min(lineno - 1); + let surrounding_src = reader.lines().skip(start_line as usize - 1).take(5); + let mut buf = String::new(); + let mut separated = f.header("\n"); + let mut f = separated.in_progress(); + for (line, cur_line_no) in surrounding_src.zip(start_line..) { + let line = line.unwrap(); + if cur_line_no == lineno { + let color = White.bold(); + write!(&mut buf, "{:>8} > {}", cur_line_no, line)?; + write!(&mut f, "{}", color.paint(&buf))?; + buf.clear(); + } else { + write!(&mut f, "{:>8} │ {}", cur_line_no, line)?; + } + f = separated.ready(); } Ok(()) @@ -187,42 +237,6 @@ impl Frame { false } - - fn print_source_if_avail(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let (lineno, filename) = match (self.lineno, self.filename.as_ref()) { - (Some(a), Some(b)) => (a, b), - // Without a line number and file name, we can't sensibly proceed. - _ => return Ok(()), - }; - - let file = match std::fs::File::open(filename) { - Ok(file) => file, - Err(ref e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(()), - e @ Err(_) => e.unwrap(), - }; - - use std::fmt::Write; - use std::io::BufRead; - - // Extract relevant lines. - let reader = std::io::BufReader::new(file); - let start_line = lineno - 2.min(lineno - 1); - let surrounding_src = reader.lines().skip(start_line as usize - 1).take(5); - let mut buf = String::new(); - for (line, cur_line_no) in surrounding_src.zip(start_line..) { - let line = line.unwrap(); - if cur_line_no == lineno { - let color = White.bold(); - write!(&mut buf, "{:>8} > {}", cur_line_no, line)?; - writeln!(f, "{}", color.paint(&buf))?; - buf.clear(); - } else { - writeln!(f, "{:>8} │ {}", cur_line_no, line)?; - } - } - - Ok(()) - } } /// Builder for customizing the behavior of the global panic and error report hooks @@ -322,10 +336,12 @@ impl HookBuilder { pub(crate) fn into_hooks(self) -> (PanicHook, EyreHook) { let panic_hook = PanicHook { filters: self.filters.into_iter().map(Into::into).collect(), + #[cfg(feature = "capture-spantrace")] capture_span_trace_by_default: self.capture_span_trace_by_default, }; let eyre_hook = EyreHook { + #[cfg(feature = "capture-spantrace")] capture_span_trace_by_default: self.capture_span_trace_by_default, }; @@ -387,72 +403,91 @@ fn install_panic_hook() { std::panic::set_hook(Box::new(|pi| eprintln!("{}", PanicPrinter(pi)))) } -fn print_panic_info(printer: &PanicPrinter<'_>, out: &mut fmt::Formatter<'_>) -> fmt::Result { - let pi = printer.0; +struct PanicMessage<'a>(&'a PanicPrinter<'a>); - writeln!(out, "{}", Red.paint("The application panicked (crashed)."))?; +impl fmt::Display for PanicMessage<'_> { + fn fmt(&self, out: &mut fmt::Formatter<'_>) -> fmt::Result { + let pi = (self.0).0; + writeln!(out, "{}", Red.paint("The application panicked (crashed)."))?; - // Print panic message. - let payload = pi - .payload() - .downcast_ref::() - .map(String::as_str) - .or_else(|| pi.payload().downcast_ref::<&str>().cloned()) - .unwrap_or(""); + // Print panic message. + let payload = pi + .payload() + .downcast_ref::() + .map(String::as_str) + .or_else(|| pi.payload().downcast_ref::<&str>().cloned()) + .unwrap_or(""); - write!(out, "Message: ")?; - writeln!(out, "{}", Cyan.paint(payload))?; + write!(out, "Message: ")?; + writeln!(out, "{}", Cyan.paint(payload))?; - // If known, print panic location. - write!(out, "Location: ")?; - if let Some(loc) = pi.location() { - write!(out, "{}", Purple.paint(loc.file()))?; - write!(out, ":")?; - writeln!(out, "{}", Purple.paint(loc.line().to_string()))?; - } else { - writeln!(out, "")?; + // If known, print panic location. + write!(out, "Location: ")?; + if let Some(loc) = pi.location() { + write!(out, "{}", Purple.paint(loc.file()))?; + write!(out, ":")?; + write!(out, "{}", Purple.paint(loc.line().to_string()))?; + } else { + write!(out, "")?; + } + + Ok(()) } +} + +fn print_panic_info(printer: &PanicPrinter<'_>, out: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(out, "{}", PanicMessage(printer))?; let v = panic_verbosity(); - // Print some info on how to increase verbosity. - if v == Verbosity::Minimal { - write!(out, "\nBacktrace omitted.\n\nRun with ")?; - write!(out, "RUST_BACKTRACE=1")?; - writeln!(out, " environment variable to display it.")?; - } else { - // This text only makes sense if frames are displayed. - write!(out, "\nRun with ")?; - write!(out, "COLORBT_SHOW_HIDDEN=1")?; - writeln!(out, " environment variable to disable frame filtering.")?; - } - if v <= Verbosity::Medium { - write!(out, "Run with ")?; - write!(out, "RUST_BACKTRACE=full")?; - writeln!(out, " to include source snippets.")?; - } - let printer = installed_printer(); + #[cfg(feature = "capture-spantrace")] + let span_trace = if printer.spantrace_capture_enabled() { + Some(tracing_error::SpanTrace::capture()) + } else { + None + }; + + let mut separated = out.header("\n\n"); + #[cfg(feature = "capture-spantrace")] { - if printer.spantrace_capture_enabled() { - let span_trace = tracing_error::SpanTrace::capture(); - write!(out, "{}", crate::writers::FormattedSpanTrace(&span_trace))?; + if let Some(span_trace) = span_trace.as_ref() { + write!( + &mut separated.ready(), + "{}", + crate::writers::FormattedSpanTrace(span_trace) + )?; } } - if panic_verbosity() != Verbosity::Minimal { + let capture_bt = v != Verbosity::Minimal; + + if capture_bt { let bt = backtrace::Backtrace::new(); - let fmt_bt = printer.format_backtrace(&bt); - writeln!(out, "\n\n{}", fmt_bt)?; + let fmted_bt = printer.format_backtrace(&bt); + write!( + indented(&mut separated.ready()).with_format(Format::Uniform { indentation: " " }), + "{}", + fmted_bt + )?; } + let env_section = EnvSection { + bt_captured: &capture_bt, + #[cfg(feature = "capture-spantrace")] + span_trace: span_trace.as_ref(), + }; + + write!(&mut separated.ready(), "{}", env_section)?; + Ok(()) } pub(crate) struct PanicHook { filters: Vec>, + #[cfg(feature = "capture-spantrace")] capture_span_trace_by_default: bool, } @@ -467,6 +502,7 @@ impl PanicHook { } } + #[cfg(feature = "capture-spantrace")] fn spantrace_capture_enabled(&self) -> bool { std::env::var("RUST_SPANTRACE") .map(|val| val != "0") @@ -475,6 +511,7 @@ impl PanicHook { } pub(crate) struct EyreHook { + #[cfg(feature = "capture-spantrace")] capture_span_trace_by_default: bool, } @@ -504,6 +541,7 @@ impl EyreHook { } } + #[cfg(feature = "capture-spantrace")] fn spantrace_capture_enabled(&self) -> bool { std::env::var("RUST_SPANTRACE") .map(|val| val != "0") @@ -518,7 +556,7 @@ pub(crate) struct BacktraceFormatter<'a> { impl fmt::Display for BacktraceFormatter<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - writeln!(f, "{:━^80}", " BACKTRACE ")?; + write!(f, "{:━^80}", " BACKTRACE ")?; // Collect frame info. let frames: Vec<_> = self @@ -547,9 +585,11 @@ impl fmt::Display for BacktraceFormatter<'_> { if filtered_frames.is_empty() { // TODO: Would probably look better centered. - return writeln!(f, ""); + return write!(f, "\n"); } + let mut separated = f.header("\n"); + // Don't let filters mess with the order. filtered_frames.sort_by_key(|x| x.n); @@ -566,7 +606,7 @@ impl fmt::Display for BacktraceFormatter<'_> { decorator = "⋮", ) ); - writeln!(f, "{}", color.paint(text))?; + write!(&mut separated.ready(), "{}", color.paint(text))?; }; } @@ -576,7 +616,7 @@ impl fmt::Display for BacktraceFormatter<'_> { if frame_delta != 0 { print_hidden!(frame_delta); } - write!(f, "{}", frame)?; + write!(&mut separated.ready(), "{}", frame)?; last_n = frame.n; } @@ -606,7 +646,7 @@ pub(crate) enum Verbosity { Full, } -fn panic_verbosity() -> Verbosity { +pub(crate) fn panic_verbosity() -> Verbosity { match env::var("RUST_BACKTRACE") { Ok(s) if s == "full" => Verbosity::Full, Ok(s) if s != "0" => Verbosity::Medium, diff --git a/src/handler.rs b/src/handler.rs index 5576f98..e4b3447 100644 --- a/src/handler.rs +++ b/src/handler.rs @@ -1,6 +1,10 @@ use crate::config::installed_printer; use crate::ColorExt; -use crate::{section::help::HelpInfo, writers::HeaderWriter, Handler}; +use crate::{ + section::help::HelpInfo, + writers::{EnvSection, WriterExt}, + Handler, +}; use ansi_term::Color::*; use backtrace::Backtrace; use indenter::{indented, Format}; @@ -50,11 +54,7 @@ impl eyre::EyreHandler for Handler { write!(indented(f).ind(n), "{}", Red.make_intense().paint(&buf))?; } - let separated = &mut HeaderWriter { - inner: &mut *f, - header: &"\n\n", - started: false, - }; + let mut separated = f.header("\n\n"); for section in self .sections @@ -72,13 +72,15 @@ impl eyre::EyreHandler for Handler { write!(separated.ready(), "{}", section)?; } + #[cfg(feature = "capture-spantrace")] + let span_trace = self + .span_trace + .as_ref() + .or_else(|| get_deepest_spantrace(error)); + #[cfg(feature = "capture-spantrace")] { - if let Some(span_trace) = self - .span_trace - .as_ref() - .or_else(|| get_deepest_spantrace(error)) - { + if let Some(span_trace) = span_trace { write!( &mut separated.ready(), "{}", @@ -95,22 +97,29 @@ impl eyre::EyreHandler for Handler { "{}", fmted_bt )?; - } else if self - .sections - .iter() - .any(|s| !matches!(s, HelpInfo::Custom(_) | HelpInfo::Error(_))) - { - writeln!(f)?; } + let f = separated.ready(); + let mut h = f.header("\n"); + let mut f = h.in_progress(); + for section in self .sections .iter() .filter(|s| !matches!(s, HelpInfo::Custom(_) | HelpInfo::Error(_))) { - write!(f, "\n{}", section)?; + write!(&mut f, "{}", section)?; + f = h.ready(); } + let env_section = EnvSection { + bt_captured: &self.backtrace.is_some(), + #[cfg(feature = "capture-spantrace")] + span_trace, + }; + + write!(&mut separated.ready(), "{}", env_section)?; + Ok(()) } } diff --git a/src/section/mod.rs b/src/section/mod.rs index bb08e49..a281922 100644 --- a/src/section/mod.rs +++ b/src/section/mod.rs @@ -1,5 +1,6 @@ //! Helpers for adding custom sections to error reports -use std::fmt::{self, Display, Write}; +use crate::writers::WriterExt; +use std::fmt::{self, Display}; pub(crate) mod help; @@ -55,17 +56,10 @@ where B: Display + Send + Sync + 'static, { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let mut headered = crate::writers::HeaderWriter { - inner: f, - header: &self.header, - started: false, - }; - - let mut headered = crate::writers::HeaderWriter { - inner: headered.ready(), - header: &"\n", - started: false, - }; + use std::fmt::Write; + let mut headered = f.header(&self.header); + let headered = headered.ready(); + let mut headered = headered.header("\n"); let mut headered = headered.ready(); diff --git a/src/writers.rs b/src/writers.rs index 65ce70b..cfa6669 100644 --- a/src/writers.rs +++ b/src/writers.rs @@ -1,24 +1,50 @@ +use crate::config::{lib_verbosity, panic_verbosity, Verbosity}; +use fmt::Write; use std::fmt::{self, Display}; #[cfg(feature = "capture-spantrace")] use tracing_error::{SpanTrace, SpanTraceStatus}; -pub(crate) struct HeaderWriter<'a, H, W> { - pub(crate) inner: W, - pub(crate) header: &'a H, - pub(crate) started: bool, +#[allow(explicit_outlives_requirements)] +pub(crate) struct HeaderWriter<'a, H, W> +where + H: ?Sized, +{ + inner: W, + header: &'a H, + started: bool, } -pub(crate) struct ReadyHeaderWriter<'a, 'b, H, W>(&'b mut HeaderWriter<'a, H, W>); +pub(crate) trait WriterExt: Sized { + fn header(self, header: &H) -> HeaderWriter<'_, H, Self>; +} -impl<'a, H, W> HeaderWriter<'a, H, W> { +impl WriterExt for W { + fn header(self, header: &H) -> HeaderWriter<'_, H, Self> { + HeaderWriter { + inner: self, + header, + started: false, + } + } +} + +pub(crate) struct ReadyHeaderWriter<'a, 'b, H: ?Sized, W>(&'b mut HeaderWriter<'a, H, W>); + +impl<'a, H: ?Sized, W> HeaderWriter<'a, H, W> { pub(crate) fn ready(&mut self) -> ReadyHeaderWriter<'a, '_, H, W> { self.started = false; ReadyHeaderWriter(self) } + + pub(crate) fn in_progress(&mut self) -> ReadyHeaderWriter<'a, '_, H, W> { + self.started = true; + + ReadyHeaderWriter(self) + } } -impl fmt::Write for ReadyHeaderWriter<'_, '_, H, W> +impl<'a, H: ?Sized, W> fmt::Write for ReadyHeaderWriter<'a, '_, H, W> where H: Display, W: fmt::Write, @@ -41,14 +67,102 @@ impl fmt::Display for FormattedSpanTrace<'_> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use indenter::indented; use indenter::Format; - use std::fmt::Write; - match self.0.status() { - SpanTraceStatus::CAPTURED => { - write!(indented(f).with_format(Format::Uniform { indentation: " " }), "{}", color_spantrace::colorize(self.0))?; - }, - SpanTraceStatus::UNSUPPORTED => write!(f, "Warning: SpanTrace capture is Unsupported.\nEnsure that you've setup an error layer and the versions match")?, - _ => (), + if self.0.status() == SpanTraceStatus::CAPTURED { + write!( + indented(f).with_format(Format::Uniform { indentation: " " }), + "{}", + color_spantrace::colorize(self.0) + )?; + } + + Ok(()) + } +} + +pub(crate) struct EnvSection<'a> { + pub(crate) bt_captured: &'a bool, + #[cfg(feature = "capture-spantrace")] + pub(crate) span_trace: Option<&'a SpanTrace>, +} + +impl fmt::Display for EnvSection<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let v = if std::thread::panicking() { + panic_verbosity() + } else { + lib_verbosity() + }; + write!(f, "{}", BacktraceOmited(!self.bt_captured))?; + + let mut separated = HeaderWriter { + inner: &mut *f, + header: &"\n", + started: false, + }; + write!(&mut separated.ready(), "{}", SourceSnippets(v))?; + #[cfg(feature = "capture-spantrace")] + write!( + &mut separated.ready(), + "{}", + SpanTraceOmited(self.span_trace) + )?; + Ok(()) + } +} + +#[cfg(feature = "capture-spantrace")] +struct SpanTraceOmited<'a>(Option<&'a SpanTrace>); + +#[cfg(feature = "capture-spantrace")] +impl fmt::Display for SpanTraceOmited<'_> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if let Some(span_trace) = self.0 { + if span_trace.status() == SpanTraceStatus::UNSUPPORTED { + writeln!(f, "Warning: SpanTrace capture is Unsupported.")?; + write!( + f, + "Ensure that you've setup an error layer and the versions match" + )?; + } + } + + Ok(()) + } +} + +struct BacktraceOmited(bool); + +impl fmt::Display for BacktraceOmited { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + // Print some info on how to increase verbosity. + if self.0 { + writeln!(f, "Backtrace omitted.")?; + write!( + f, + "Run with RUST_BACKTRACE=1 environment variable to display it." + )?; + } else { + // This text only makes sense if frames are displayed. + write!( + f, + "Run with COLORBT_SHOW_HIDDEN=1 environment variable to disable frame filtering." + )?; + } + + Ok(()) + } +} + +struct SourceSnippets(Verbosity); + +impl fmt::Display for SourceSnippets { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + if self.0 <= Verbosity::Medium { + write!( + f, + "Run with RUST_BACKTRACE=full to include source snippets." + )?; } Ok(())