Skip to content

Commit

Permalink
subscriber: update pretty formatter for no ansi
Browse files Browse the repository at this point in the history
 ## 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 `<Format<Pretty, T> as FormatEvent<C, N>>::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, tokio-rs#658 is worth investigating further.

Refs: tokio-rs#658
  • Loading branch information
katelyn martin committed Mar 11, 2021
1 parent ba57971 commit 222baab
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 10 deletions.
7 changes: 7 additions & 0 deletions tracing-subscriber/src/fmt/format/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
49 changes: 39 additions & 10 deletions tracing-subscriber/src/fmt/format/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -232,6 +247,7 @@ impl<'a> PrettyVisitor<'a> {
Self {
writer,
is_empty,
ansi: true,
style: Style::default(),
result: Ok(()),
}
Expand All @@ -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;
Expand All @@ -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!(
Expand All @@ -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,),
Expand Down Expand Up @@ -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()
}
}

0 comments on commit 222baab

Please sign in to comment.