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

judemille · 2020-10-03T21:13:16Z

Fixes #184, makes text readable no matter the color scheme.

IceSentry · 2020-10-05T02:42:43Z

src/encode/pattern/mod.rs

@@ -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))?,


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

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

I would prefer to have us in parity with env_logger.

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

Wait nevermind, it has cyan.

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.

ray-kast · 2020-10-05T22:22:41Z

I disagree with using Style::new().intense(true) as the trace format, as some terminals will also interpret this as "use a bold font" in addition to "use bright white." If the format pattern included {h({m})} or any other pattern that expands to a long string, this would result in a lot of bold, bright-white text on the terminal given the typical volume of trace log records. Something like Style::new().text(Color::Blue) would be much easier on the eyes, and still convey the same information.

IceSentry · 2020-10-05T22:57:29Z

@Rookie1024 I'd rather trace have no specific color at this point until this is made configurable. I don't think going from no color but intense to blue is a solution to the problem, but it's a valid concern that intense might not be ideal for trace level

ray-kast · 2020-10-05T23:27:47Z

@IceSentry Ah, that's fair. I was worried this was going to remain unconfigurable, which would have made this PR a nightmare for my current use case where I am actually using {h({m})} because I suck at reading :p

estk · 2020-10-08T06:51:46Z

src/encode/pattern/mod.rs

-                    Level::Error | Level::Warn | Level::Info => w.set_style(&Style::new())?,
+                    Level::Error | Level::Warn | Level::Info | Level::Trace => {
+                        w.set_style(&Style::new())?
+                    }


why are we adding trace level here again?

estk · 2020-12-16T02:37:50Z

@judemille we're so close! Any update on this?

judemille · 2020-12-16T02:38:54Z

I think someone just wanted to add the trace level? Not sure, just this is my best take on making what existed compatible.

E: Right, in that second statement, it's to clear the intense set earlier in the file.

estk · 2020-12-16T02:45:37Z

@judemille I think the only thing left is just using the colors in #190 so that it will match env_logger

estk · 2020-12-16T02:49:41Z

@judemille what envlogger sets:

        match level {
            Level::Trace => level_style.set_color(Color::Cyan),
            Level::Debug => level_style.set_color(Color::Blue),
            Level::Info => level_style.set_color(Color::Green),
            Level::Warn => level_style.set_color(Color::Yellow),
            Level::Error => level_style.set_color(Color::Red).set_bold(true),
        };

E: I think just fixing trace is good actually

* Expose errors as failure::Error (#138) * Issue #129: Drop XML Config support (#137) * Rename file feature (#147) * Rename to config_parsing * Add workflows for devel * Update changelog * Add an init function which takes a RawConfig (#150) * Clippy * Changelog, cleanup * json macro * qualify * typo * Remove unused dir from .gitignore * Init raw on all platforms * Remove log4rs::FormatError::XmlFeatureFlagRequired (#156) * Expand env vars in the path for File and RollingFile appenders (#155) * Reorganize config (#157) * Reorganize config * allow missing docs so tests run * lint abatement * some renames * Use anyhow/thiserror (#159) * Use anyhow/thiserror * Change addtivity to additive to match the actual code (#163) * Errors 1.0 (#160) Co-authored-by: shmapdy <richard.meester@gmail.com> * clippy * Standard derives rebase (#175) * Bump serde-value * Alpha 1 version * Update highlight colors to be the same as env_logger (#167) * Custom err handler (#183) * WIP custom err handling * Compiling * Add an method to the public api * re-enable Debug * bump ver * fix: init_raw_config forcing max_log_level to Info (#200) Co-authored-by: braindead <braindeaded@protonmail.com> * pattern encoder: Set trace to default color, reset formatting after (#186) * pattern encoder: Set trace to default color, reset formatting after * Fix formatting of last commit * lol apparently rustfmt doesn't like commas? idk if this will work * Set trace color to cyan Co-authored-by: Richard M <gadunga@users.noreply.github.com> Co-authored-by: shmapdy <richard.meester@gmail.com> Co-authored-by: Charles Giguere <IceSentry@users.noreply.github.com> Co-authored-by: 1c7718e7 <shirokovMartin@gmail.com> Co-authored-by: braindead <braindeaded@protonmail.com> Co-authored-by: Julia DeMille <8127111+judemille@users.noreply.github.com>

judemille added 3 commits October 3, 2020 16:12

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

18da9c0

Fix formatting of last commit

c4b467f

lol apparently rustfmt doesn't like commas? idk if this will work

69d7c14

IceSentry approved these changes Oct 5, 2020

View reviewed changes

judemille mentioned this pull request Oct 7, 2020

Change level colors to match env_logger #190

Closed

estk reviewed Oct 8, 2020

View reviewed changes

noyez mentioned this pull request Oct 14, 2020

configurable highlight #194

Closed

Set trace color to cyan

6aee909

estk merged commit 36627ba into estk:devel Dec 16, 2020

noyez mentioned this pull request Dec 28, 2020

WIP: Adding ability to configure colors for console #204

Closed

estk mentioned this pull request Mar 30, 2021

Highlight doesn't reset style for trace/trace color is unreadable #184

Closed

noyez mentioned this pull request Jul 10, 2021

Adding ability to configure colors for console #224

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

judemille commented Oct 3, 2020

IceSentry Oct 5, 2020

judemille Oct 5, 2020

estk Oct 6, 2020

judemille Oct 7, 2020

judemille Oct 7, 2020

IceSentry Oct 7, 2020

ray-kast commented Oct 5, 2020

IceSentry commented Oct 5, 2020

ray-kast commented Oct 5, 2020

estk Oct 8, 2020

estk commented Dec 16, 2020

judemille commented Dec 16, 2020 •

edited

Loading

estk commented Dec 16, 2020

estk commented Dec 16, 2020 •

edited

Loading

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

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

Conversation

judemille commented Oct 3, 2020

IceSentry Oct 5, 2020

Choose a reason for hiding this comment

judemille Oct 5, 2020

Choose a reason for hiding this comment

estk Oct 6, 2020

Choose a reason for hiding this comment

judemille Oct 7, 2020

Choose a reason for hiding this comment

judemille Oct 7, 2020

Choose a reason for hiding this comment

IceSentry Oct 7, 2020

Choose a reason for hiding this comment

ray-kast commented Oct 5, 2020

IceSentry commented Oct 5, 2020

ray-kast commented Oct 5, 2020

estk Oct 8, 2020

Choose a reason for hiding this comment

estk commented Dec 16, 2020

judemille commented Dec 16, 2020 • edited Loading

estk commented Dec 16, 2020

estk commented Dec 16, 2020 • edited Loading

judemille commented Dec 16, 2020 •

edited

Loading

estk commented Dec 16, 2020 •

edited

Loading