From 222baababae6cac21a1b6462df3ab652e5b85ebd Mon Sep 17 00:00:00 2001 From: katelyn martin Date: Mon, 15 Feb 2021 16:12:59 -0500 Subject: [PATCH] subscriber: update pretty formatter for no ansi ## Background Currently, when the `Pretty` event formatter is being used, it does not change its output when the `with_ansi` flag is set to false by the `CollectorBuilder`. ## Overview While this formatter is generally used in situations such as local development, where ANSI escape codes are more often acceptable, there are some situations in which this can lead to mangled output. This commit makes some minor changes to account for this `ansi` flag when formatting events using `Pretty`. Becuase ANSI codes were previously used to imply the event level using colors, this commit additionally modifies `Pretty` so that it respects `display_level` when formatting an event. ## Changes * Changes to ` as FormatEvent>::format_event` * Implement `LevelNames` for `Pretty`, copying `Full`'s implementation. * Add a `PrettyVisitor::ansi` boolean flag, used in its `Visit` implementation. * Add a new `PrettyVisitor::with_ansi` builder pattern method to facilitate this. ## Out of Scope One detail worth nothing is that this does not solve the problem of *fields* being formatted without ANSI codes. Configuring a subscriber using this snippet would still lead to bolded fields in parent spans. ```rust tracing_subscriber::fmt() .pretty() .with_ansi(false) .with_level(false) .with_max_level(tracing::Level::TRACE) .init(); ``` This can be worked around by using a different field formatter, via `.fmt_fields(tracing_subscriber::fmt::format::DefaultFields::new())` in the short-term. In the long-term, #658 is worth investigating further. Refs: #658 --- tracing-subscriber/src/fmt/format/mod.rs | 7 +++ tracing-subscriber/src/fmt/format/pretty.rs | 49 ++++++++++++++++----- 2 files changed, 46 insertions(+), 10 deletions(-) diff --git a/tracing-subscriber/src/fmt/format/mod.rs b/tracing-subscriber/src/fmt/format/mod.rs index f33b8c85c8..14d62d1cb6 100644 --- a/tracing-subscriber/src/fmt/format/mod.rs +++ b/tracing-subscriber/src/fmt/format/mod.rs @@ -935,6 +935,13 @@ trait LevelNames { } } +impl LevelNames for Pretty { + const TRACE_STR: &'static str = "TRACE"; + const DEBUG_STR: &'static str = "DEBUG"; + const INFO_STR: &'static str = " INFO"; + const WARN_STR: &'static str = " WARN"; + const ERROR_STR: &'static str = "ERROR"; +} impl LevelNames for Full { const TRACE_STR: &'static str = "TRACE"; const DEBUG_STR: &'static str = "DEBUG"; diff --git a/tracing-subscriber/src/fmt/format/pretty.rs b/tracing-subscriber/src/fmt/format/pretty.rs index 3aa94b9b8b..a8664bac99 100644 --- a/tracing-subscriber/src/fmt/format/pretty.rs +++ b/tracing-subscriber/src/fmt/format/pretty.rs @@ -32,6 +32,7 @@ pub struct Pretty { pub struct PrettyVisitor<'a> { writer: &'a mut dyn Write, is_empty: bool, + ansi: bool, style: Style, result: fmt::Result, } @@ -98,30 +99,40 @@ where #[cfg(not(feature = "tracing-log"))] let meta = event.metadata(); write!(writer, " ")?; - time::write(&self.timer, writer, true)?; + time::write(&self.timer, writer, self.ansi)?; - let style = if self.display_level { + let style = if self.display_level && self.ansi { Pretty::style_for(meta.level()) } else { Style::new() }; + if self.display_level { + self.format_level(*meta.level(), writer)?; + } + if self.display_target { - let bold = style.bold(); + let target_style = if self.ansi { style.bold() } else { style }; write!( writer, "{}{}{}: ", - bold.prefix(), + target_style.prefix(), meta.target(), - bold.infix(style) + target_style.infix(style) )?; } - let mut v = PrettyVisitor::new(writer, true).with_style(style); + let mut v = PrettyVisitor::new(writer, true) + .with_style(style) + .with_ansi(self.ansi); event.record(&mut v); v.finish()?; writer.write_char('\n')?; - let dimmed = Style::new().dimmed().italic(); + let dimmed = if self.ansi { + Style::new().dimmed().italic() + } else { + Style::new() + }; let thread = self.display_thread_name || self.display_thread_id; if let (true, Some(file), Some(line)) = (self.format.display_location, meta.file(), meta.line()) @@ -156,7 +167,11 @@ where writer.write_char('\n')?; } - let bold = Style::new().bold(); + let bold = if self.ansi { + Style::new().bold() + } else { + Style::new() + }; let span = event .parent() .and_then(|id| ctx.span(&id)) @@ -232,6 +247,7 @@ impl<'a> PrettyVisitor<'a> { Self { writer, is_empty, + ansi: true, style: Style::default(), result: Ok(()), } @@ -241,6 +257,10 @@ impl<'a> PrettyVisitor<'a> { Self { style, ..self } } + pub(crate) fn with_ansi(self, ansi: bool) -> Self { + Self { ansi, ..self } + } + fn maybe_pad(&mut self) { if self.is_empty { self.is_empty = false; @@ -265,7 +285,11 @@ impl<'a> field::Visit for PrettyVisitor<'a> { fn record_error(&mut self, field: &Field, value: &(dyn std::error::Error + 'static)) { if let Some(source) = value.source() { - let bold = self.style.bold(); + let bold = if self.ansi { + self.style.bold() + } else { + Style::new() + }; self.record_debug( field, &format_args!( @@ -286,7 +310,11 @@ impl<'a> field::Visit for PrettyVisitor<'a> { if self.result.is_err() { return; } - let bold = self.style.bold(); + let bold = if self.ansi { + self.style.bold() + } else { + Style::new() + }; self.maybe_pad(); self.result = match field.name() { "message" => write!(self.writer, "{}{:?}", self.style.prefix(), value,), @@ -333,6 +361,7 @@ impl<'a> fmt::Debug for PrettyVisitor<'a> { .field("is_empty", &self.is_empty) .field("result", &self.result) .field("style", &self.style) + .field("ansi", &self.ansi) .finish() } }