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.

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

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

Open
hoijui opened this issue Aug 23, 2021 · 5 comments · May be fixed by #158
Open

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

hoijui opened this issue Aug 23, 2021 · 5 comments · May be fixed by #158

Comments

@hoijui
Copy link

hoijui commented Aug 23, 2021

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
Copy link
Owner

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
Copy link
Author

hoijui commented Aug 23, 2021

: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
Copy link
Owner

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
Copy link
Author

hoijui commented Aug 24, 2021

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants