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

Add support for writing to streams other than standard output and standard error. #62

Closed
wants to merge 1 commit into from

Conversation

SolraBizna
Copy link

This PR adds generic versions of BufferWriter and StandardStream that can be used with any Write implementation. They will output color only if explicitly requested (ColorChoice::Always or ColorChoice::AlwaysAnsi), and will always use ANSI codes to do it, regardless of platform. This is useful for any situation where you might want to output ANSI escape sequences to a location other than standard output or standard error: other file descriptors, serial terminals, ANSI art files, etc.

(This is created specifically in response to env_logger issue #208. env_logger's pervasive use of termcolor::BufferWriter greatly complicates adding support for destinations other than standard output and standard error, with or without color.)

@John-Nagle
Copy link

John-Nagle commented Sep 27, 2022

Definitely want this in env_logger. As someone wrote, "Make your own logger" isn't a good solution, because then we give up env_logger's flagship (and titular) feature, which is its simple and powerful system by which the user specifies which log messages they're interested in.

I'm debugging a large GUI program, a metaverse client. If I turn on logging program-wide at the INFO level, I get gigabytes of log file, because WGPU writes log messages on every frame. So I badly need selective logging.

Using tracing has been suggested. But I already have tracy working for analyzing performance problems. Using tracing just to move the output from stderr to a file is overkill.

The env_logger people really seem to be into colored output. They took out the ability to write to a pipe because they couldn't figure out how to color it.

@BurntSushi
Copy link
Owner

So I badly need selective logging.

env_logger has this via the RUST_LOG environment variable, so I don't know what you're talking about. Consider posting to the user forum or filing an issue with env_logger and include real code samples that others can run to reproduce your problem.

@BurntSushi
Copy link
Owner

@SolraBizna Thanks for this PR. It would be good to discuss such changes conceptually via an issue before filing a PR though, in order to save work. In particular, I think I'm going to pass on this. This feature is IMO too big of a scope increase from what termcolor is designed to do: provide a cross platform abstraction for colorizing output. This PR adds more APIs that are not cross platform. If you don't care about cross platform support (and it is legitimate not to, given that Windows has support ANSI colors for a few years now), then it doesn't make sense to use termcolor's cumbersome API.

The fix here is to refactor env_logger. You might even consider abandoning color support for Windows consoles, in which case, you can double down on ANSI coloring everywhere and I imagine things will become quite a bit easier. Otherwise, you'll need to introduce some abstraction layer between env_logger and BufferWriter.

@BurntSushi BurntSushi closed this Sep 27, 2022
@SolraBizna
Copy link
Author

@SolraBizna Thanks for this PR. It would be good to discuss such changes conceptually via an issue before filing a PR though, in order to save work. In particular, I think I'm going to pass on this. This feature is IMO too big of a scope increase from what termcolor is designed to do: provide a cross platform abstraction for colorizing output. This PR adds more APIs that are not cross platform. If you don't care about cross platform support (and it is legitimate not to, given that Windows has support ANSI colors for a few years now), then it doesn't make sense to use termcolor's cumbersome API.

The fix here is to refactor env_logger. You might even consider abandoning color support for Windows consoles, in which case, you can double down on ANSI coloring everywhere and I imagine things will become quite a bit easier. Otherwise, you'll need to introduce some abstraction layer between env_logger and BufferWriter.

Obviously this is your project, and you're the one who decides what does and does not go in. But I'm extremely confused by your assertion that the PR adds "more APIs that are not cross platform". It adds a single target that has no platform-specific dependencies and will work exactly the same way on any platform. The PR is 99% plumbing; it adds basically no logic, and conceptually the new target is very simple. And, with it, an application that wants to be able to "output text, but in color where possible" no longer needs to write an entire abstraction layer encompassing all of termcolor's functionality just to be able to output non-colored text to places other than stdout or stderr with the same code.

And for the record, I have nothing to do with the env_logger project, but if I did, I wouldn't consider "abandoning color support for Windows consoles", let alone switching to hardcoding ANSI control sequences. I'm already annoyed enough that nobody supports non-ANSI, non-Windows terminals (though obviously not annoyed enough to do something about it 🙂).

@BurntSushi
Copy link
Owner

I don't know what you're talking about with respect to non-ANSI and non-Windows-console colors. I'm not aware of any other mechanism for coloring in a terminal.

The "cross platform" is in reference to supporting colors. ANSI-only is not cross platform in that respect, because it does not work in Windows consoles that don't support ANSI coloring. The Windows console APIs only work on stdout and stderr as far as I'm aware.

termcolor is not a crate for ANSI coloring alone. Its whole point is to provide an abstraction that works for both ANSI and Windows consoles that don't support ANSI. As far as I can tell, this PR adds new APIs that only support ANSI coloring.

And for the record, I have nothing to do with the env_logger project

I didn't say you did? Your PR comment mentions env_logger specifically as motivation for this PR, so that's what I responded to.......

@SolraBizna
Copy link
Author

I don't know what you're talking about with respect to non-ANSI and non-Windows-console colors. I'm not aware of any other mechanism for coloring in a terminal.

In my own crate that runs into this issue, I have support for VT52s as well. Those don't support ANSI anything. I come by it because the Atari ST implements a color-capable VT52 emulator in ROM. That was even more of a head-scratcher then than now; even at the time, the VT52 was already considered obsolete legacy kit.

The "cross platform" is in reference to supporting colors. ANSI-only is not cross platform in that respect, because it does not work in Windows consoles that don't support ANSI coloring. The Windows console APIs only work on stdout and stderr as far as I'm aware.

termcolor is not a crate for ANSI coloring alone. Its whole point is to provide an abstraction that works for both ANSI and Windows consoles that don't support ANSI. As far as I can tell, this PR adds new APIs that only support ANSI coloring.

The main purpose was to support outputting uncolored text to a buffer or pipe, same as if you were outputting to a non-color-capable terminal. It only supported optionally outputting ANSI because that was actually a little simpler than adding an uncolored-only target.

I didn't say you did? Your PR comment mentions env_logger specifically as motivation for this PR, so that's what I responded to.......

Sorry that I came off as hostile.

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.

None yet

3 participants