add configure method, to enable users to own a valid configured instance before calling `set_boxed_logger`

[twiby]

My personal use case is to wrap this logger in my own logger implementation, to be able to catch log calls during testing and react to them. This PR proposes to add a configure method to enable users to have a completely configured instance of SimpleLogger. Then they can set as global logger something else that might be a wrapper around SimpleLogger.

Usage is simply to call configure instead of init to finish configuration of an instance of SimpleLogger. To avoid code duplication, init will internally call configure before set_boxed_logger. This means that using the chain simpleLogger::new().configure().init() has bad performance and would not be advised. A user would either use configure or init.

Another implentation would be to add a boolean argument to init, but I didn't want to change usual behavior/usage.

If this is not interesting, please ignore/close this PR, I'll continue using my fork

[borntyping]

I think it definitely makes sense to be able to wrap SimpleLogger like this.

I wonder if it might make sense to break out the specific things init() does instead of splitting it into configure()? So it might have a set_up_color_terminal() method† and a max_level() method that could be called by someone making their own calls to log::set_boxed_logger and log::set_max_level. I don't really like the idea that SimpleLogger would partially configure log::, I feel like that should be entirely in control of the caller or controlled by init(). I'm not set on any of this—how does it sound to you?

(†I guess the existing set_up_color_terminal() could just be made public with the addition of a stub for non-windows builds.)

[twiby]

Hi, thank you for answering ! I think what you're saying makes a lot of sense, especially configure log:: entirely or not at all. I've pushed my proposed changes: configure is still there but returns the max_level variable. The API is still kept at a minimum that way. I could also add an additional max_level method.

Should I also include a README update in the PR to explain the new feature ?

[borntyping]

Yes - it's definitely worth including something in the README about this or adding an example in the examples directory :)

I think it might make sense to rename configure to max_level. I'm happy to look at splitting the windows terminal setup bit out myself if you want (and/or adding to the README).

[twiby]

Hi ! I'll try and add something to the README.

I'm hesitant to rename configure to max_level because this function actually does stuff, and the name max_level would suggest only a simple getter that people could ignore (which they shouldn't).

Let's decide on a suitable API to be sure that I understand you. Are you suggesting the user do:

let mut logger = SimpleLogger::new();
set_up_color_terminal();
let max_level = logger.max_level();

? And then of course the user has its own code that uses something else as a logger?

The current branch state makes the user do:

let mut logger = simpleLogger::new();
let max_level = logger.configure();

[twiby]

I should add that I'm completely fine if you wanna do those changes yourself of course, if you have a precise idea of what you want

[borntyping]

Let's decide on a suitable API to be sure that I understand you. Are you suggesting the user do:

let mut logger = SimpleLogger::new();
set_up_color_terminal();
let max_level = logger.max_level();

Yeah, this is exactly what I was thinking – I should have included an example. I see init() as calling managing various bits of external state, and if a user wants to be able to do things differently they should get full control.

[borntyping]

(I might get a chance to work on this today, I'll note on this PR if I start doing any work on it.)

[twiby]

I've updated the PR with your suggestion, and also added a paragraph to the README.

Do not hesitate to modify what I've proposed if you think of something better (especially the README part, I'm not sure it's super clear).

[borntyping]

Released as part of 4.3.0, thanks!