log4rs icon indicating copy to clipboard operation
log4rs copied to clipboard

Adding ability to configure colors for console

Open noyez opened this issue 4 years ago • 11 comments

(reopening from PR:#204, now includes tests and docs)

Specifically this useful for dark console backgrounds where the default for 'Blue' for log level INFO is difficult to read.

Most of the changes are in src/encode/patter/mod.rs. I added default parameters for deserializing. I added a function to describe the default pattern string, and defined that at the top of the source file. I added a color_map to the Encoder structs which is a HashMap that defines the LogLovel->Color mapping. The color_map is of type HashMap<Level, Option<Color>>, if the Option is None, then no styling is applied.

Docs were added to src/encode/pattern/mod.rs to the function new_with_colormap() Also, the test function was added fn check_color_hash() to src/encode/pattern/mod.rs which tests a single change to the default color-map then checks the resulting color-map to make that change was made and that remain default colors are unchanged.

TODO/Future: This patch only allows foreground color to be specified. There are other aspects of styling like bold,italics,background color. Does it make sense to allow for more detailed styling? That involves more work and more work for the maintainer. I think foreground color will probably cover most everyone's cases.

To use, the relevant config change is as follows:

  pattern: "%m"
  color_map:
    INFO: Blue
    TRACE: Black

Related: issue:#17, PR:#186, PR:#204 Continued from: PR:#194

noyez avatar Jul 10 '21 00:07 noyez

Hi @noyez, First I think we'll need to cherry pick your commits onto master to eliminate spurious diffs.

estk avatar Jul 13 '21 15:07 estk

@noyez bump

estk avatar Apr 30 '22 02:04 estk

Just merged code w/ v1.1.1.

Was there something missing on this patch that you'd like to see before it gets merged? I thought it was ready go to, but if want to see something more or something changed let me know and i'll accommodate.

noyez avatar Apr 30 '22 14:04 noyez

Thanks @noyez I'll review and let you know. Let's start by getting the CI passing.

estk avatar Apr 30 '22 14:04 estk

I left a few comments re. changes that will block us from merging for semver reasons. Also, I had a more general thought: I think that instead of using a HashMap<Level, Option<Color>>, we should probably define a

struct ColorMap {
 info: Color,
 ...
}

#[derive(SmartDefault,...)] 
enum Color {
 #[default]
 Black,
...
}

Then here we can write something structurally correct rather than just procedurally. Does that make sense @noyez ?

estk avatar Apr 30 '22 14:04 estk

Does that make sense @noyez ?

Totally, i can work on this. not today but soon. This approach makes sense and i agree that its probably a better way to handle it instead of using hash map. it shouldn't take much time to make this change.

noyez avatar Apr 30 '22 15:04 noyez

@noyez thanks for that, and thanks for the work on this!

estk avatar Apr 30 '22 15:04 estk

Ok, now i believe its ready for your review. Pardon my simple usage of github's PRs. I created a new struct ColorMap, its very basic and simple. It has methods for set(level, color), get(level) and unset(level), the latter just sets the styling to None, which is no styling. I'm not sure this is the best interface, you may want to tweak it, just lmk and i will do it. I did not use SmartDefault since writing a default for the color map is pretty trivial, and SmartDefault wasn't an existing dependency. I tried to add some documentation, but i'm not the best with rustdoc. Its all relatively crude, but its also relatively simple so i didn't see the need to complicate it.

noyez avatar May 05 '22 21:05 noyez

What is the right syntax to add color? I tried finding it in documentation but wasn't able to 😔. Can someone please help me with that.

akashlama1998-icloud avatar Oct 01 '22 15:10 akashlama1998-icloud

You can use {h({l})} to add color. You can check the documents @akashlama1998-icloud

Dirreke avatar Dec 07 '22 12:12 Dirreke

Progess @noyez @estk @akashlama1998-icloud @Dirreke @dclause ?

Creative-Difficulty avatar Mar 07 '24 14:03 Creative-Difficulty