Drakulix / simplelog.rs

Simple Logging Facility for Rust
https://docs.rs/simplelog/
Apache License 2.0
423 stars 71 forks source link

`TerminalMode::MixedWarnings` (Errors *and Warnings* go to `stderr`) #78

Open hoijui opened 3 years ago

hoijui commented 3 years ago

I would like to have an other mode (have no good idea for the mode name) that does that - Warnings and Errors to stderr, everything else to stdout. TerminalMode::Mixed splits on Errors, this would split on Warnings.

I imagine I could implement it myself, if you would prefer that, and would accept a PR for it.

Drakulix commented 3 years ago

Nice proposal. Given the amount of customization simplelog already allows, we should probably support more crazy use-cases for TerminalMode as well. But not by supporting just one more MixedWarnings mode.

The way I would like to see this implemented would be something like a TerminalMode::Custom, which allows you do specify per log level, where the output should go. So that would look something like this:

pub enum Target /* TODO: bikeshed over this name */ {
  Stdout,
  Stderr,
}

pub enum TerminalMode {
    Stdout,
    Stderr,
    Mixed,
    Custom {
      error: Target,
      warn: Target,
      info: Target,
      debug: Target,
      trace: Target,
    },
}

With that done, we also might want to deprecate Mixed mode, as users could just use Custom in the future, while Stdout and Stderr provide sensible shortcuts. Because Mixed has long been the default, I would not remove it, but rather give it a proper deprecation notice. Given that TerminalMode was not marked as non_exhaustive (and does not need to be after the change), this will require a version bumb anyway.

If that sounds like a good idea to you, please start working on a PR. If you have any questions, feel free to ping me.

hoijui commented 3 years ago

:D ahaa.. someone comes and offers free help, and you ask for more! .. no sorry... that makes no sense to me. why would anyone want such control? I can't imagine such a scenario, and I have too mch work already.

Drakulix commented 3 years ago

I get that, but while splitting at warnings instead of errors, like you are proposing, sounds more reasonable on paper, the next person wants to split on Info for some reason. And who am I to say they are wrong? The only real solution to please everyone is just to make this customizable.

Anyway, I get if you do not feel like implementing this. I will leave the issue open and maybe come around to doing it myself at some point. But I cannot promise anything :)

hoijui commented 3 years ago

I understand that reasoning. There is nobody around who wants that though, and I know it as an anti-pattern to implement stuff (in a much more complex way), to suit needs that are purely imagined. Also, if need for such a solution arises, it can still be done, and I think it makes sense to have shortcuts for both Mixed and MixedWarnings even then. Either way, it's your thing, and I' just use an other log implementation then (probably slog).

Drakulix commented 3 years ago

I know it as an anti-pattern to implement stuff (in a much more complex way), to suit needs that are purely imagined.

While I do agree with that line of reasoning in generell, I do not think an enum variant with five boolean flags really introduces unneeded complexity. The code has to check the LogLevel anyway to decide where to print. This is complexity simplelog arguably already has and just does not expose to the user.

Either way, it's your thing, and I' just use an other log implementation then (probably slog).

Fair. Thanks for your time anyway. I think these kinds of discussions are healthy for the project and while I do not always agree with suggestions, I am thankful for the feedback.