Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pattern encoder: Set trace to default color, reset formatting after #186

Merged
merged 4 commits into from
Dec 16, 2020
Merged

pattern encoder: Set trace to default color, reset formatting after #186

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
18da9c0
pattern encoder: Set trace to default color, reset formatting after
judemille Oct 3, 2020
c4b467f
Fix formatting of last commit
judemille Oct 3, 2020
69d7c14
lol apparently rustfmt doesn't like commas? idk if this will work
judemille Oct 3, 2020
6aee909
Set trace color to cyan
judemille Dec 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/encode/pattern/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,14 +584,16 @@ impl FormattedChunk {
}
Level::Warn => w.set_style(Style::new().text(Color::Yellow))?,
Level::Info => w.set_style(Style::new().text(Color::Green))?,
Level::Trace => w.set_style(Style::new().text(Color::Black).intense(true))?,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could probably remove the .intense or the branch altogether I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, I'd like to keep the intense so it's more clear it's a logging level.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer to have us in parity with env_logger.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm understanding correctly, the color in env_logger causes issues, at least on my terminal.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait nevermind, it has cyan.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, in the source code of env_logger it does set the default color of trace to Cyan, but I just can't get it to print Cyan, but every other color is the same as what's printed. I distinctly remember seeing something very similar to (Color::Black).intense(true) when I originally made the PR and was the reason why I used intense here, but I just can't find it in their source code right now.

Level::Trace => w.set_style(Style::new().text(Color::Cyan))?,
_ => {}
}
for chunk in chunks {
chunk.encode(w, record)?;
}
match record.level() {
Level::Error | Level::Warn | Level::Info => w.set_style(&Style::new())?,
Level::Error | Level::Warn | Level::Info | Level::Trace => {
w.set_style(&Style::new())?
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why are we adding trace level here again?

_ => {}
}
Ok(())
Expand Down