Skip to content

Increased default logging level to info unless RUST_LOG is set #369

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

Merged
merged 1 commit into from
Jul 8, 2017

Conversation

budziq
Copy link
Contributor

@budziq budziq commented Jun 28, 2017

Sets the INFO as the default logging level shown to stdout unless RUST_LOG env var is set.
fixes https://github.com/azerupi/mdBook/issues/358

TLNR:

  • we can merge this PR (no colors and verbose output)
  • if we can live with colored output working only on unix'y systems then adding it will take ~10lines with ansi_term
  • if we want colors but can live without support for RUST_LOG we can use simplelog
  • we might leave the logging as is and add "cargo like" split logging based on termcolor or term for the mdBook binary commands and error output)

Discussion:

I'm not really happy with the effects of this PR. The output starts to be overly verbose.
The original suggestion to use pretty_env_logger was misplaced due to two problems:

  • it does not expose the LogBuilder::filter so we cannot set the default level
  • it uses ansi_term for coloring which will not work on no-unix platforms

While the first is trivially circumvented by using env_logger directly, the second is not solvable if we want to stick to env_logger or any of its derivatives as these are incompatible with portable colored printing (all of them take preformatted string while on windows you need to directly manipulate terminal and stdio to have colored output)

On the other hand other colored loggers don't support log triggering/filtering based on RUST_LOG which might be important for consistency.

Cargo uses term while ripgrep uses termcolor crate to achieve colors in a portable manner.

We could use one of these in our own logger (or a backend for one of the more advanced loggin frameworks like fern or log4rs) But it feels too involved and we would need to reimplement env_logger's env var parsing.

All of these approaches seam forced at best and now I can see why cargo is using split logging (1 standard log for library part and 2 direct printing with termcolor for direct printing to stdout of cargo binary messages)

@azerupi
Copy link
Contributor

azerupi commented Jun 29, 2017

I'm not really happy with the effects of this PR. The output starts to be overly verbose.

We can change that by using using info, warn and error sparingly. I think there are a lot of messages that can be moved from info to debug to hide them. I will check this a little later.

I agree! Coloured output should work on all the platforms we support, I don't want to play favourites. 😊 I'm fine with having uncoloured output for now until we find a better solution.

@azerupi azerupi merged commit 325458c into rust-lang:master Jul 8, 2017
Ruin0x11 pushed a commit to Ruin0x11/mdBook that referenced this pull request Aug 30, 2020
Increased default logging level to info unless RUST_LOG is set
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 this pull request may close these issues.

Logging to stdout seems to be disabled
2 participants