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

tracing-subscriber should not use nu-ansi-term and prefer another crate like owo_colors or colored #2759

Open
bzm3r opened this issue Oct 13, 2023 · 5 comments · May be fixed by #2914
Open

Comments

@bzm3r
Copy link

bzm3r commented Oct 13, 2023

Feature Request

tracing-subscriber should use another coloring crate, rather than nu-ansi-term (for example: termcolor, owo-colors, or colored)

Crates

tracing-subscriber

Motivation

Because:

  • tracing-subscriber, being part of tracing, and ultimately the tokio project, is influential on those who do not know the details of Rust's crate ecosystem, and might think that nu-ansi-term is a good choice for styling terminal output
  • it seems nu-ansi-term is used mostly because it provides continuity from ansi-term
  • nu-ansi-term, inherited from ansi_term before it, has questionable design decisions which do not make it easy to write ANSI output to terminal with zero additional allocation (for example, format_args! cannot be styled, and adding that functionality to nu-ansi-term is not easy, for a few different reasons, but here are a couple that stick out to me:
    • AnyWrite seems to be a design error because it is not necessary to be generic over both io::Write and fmt::Write, as we are only interested in printing out string/formattable/utf-8 data; compare instead with owo_color's approach, where formatters (which can write to any stream willing to present itself as an fmt::Write implementor)
    • there is a lot of unnecessary, code duplication, making it more error prone to develop for nu-ansi-term; due to their design principles nu-ansi-term wants to avoid using macros, which also makes it more difficult to extend
  • other performance reasons: apart from nu-ansi-term not making it easy to write ANSI styled output with zero-allocation, write! is often used where it doesn't need to be, and write_str would suffice --- I think the reason why write! is used is because when trying to use the write_str method that AnyWrite provides in that context, one runs into issues specifying which write_str to call: io::Writer's impl of AnyWrite (which uses its write_all under the hood), or fmt::Writer's (which uses its write_str under the hood)?

Proposal

tracing-subscriber should use another coloring crate, rather than nu-ansi-term (for example: termcolor, owo-colors, or colored)

See here for more discussion on the technical pros/cons of each choice: nicoburns/blessed-rs#100

Alternatives

The primary alternative is to make upstream changes to nu-ansi-term. It requires spending a lot of time to re-produce work that others have done, even better. Furthermore, the nu-ansi-team has concerns of its own around such upgrades, such as requiring backwards compatibility.

@hawkw
Copy link
Member

hawkw commented Oct 13, 2023

Since the dependency on nu-ansi-term is not exposed in public APIs, I would happily accept a PR that replaces it with a different crate, provided that the replacement also supports tracing-subscriber's minimum supported Rust version.

@stefnotch
Copy link

I took a stab at using owo-colors, but it seems like it's not quite as straightforward jam1garner/owo-colors#123

I believe the same issue would also apply to most other coloring crates. I wonder what the correct fix would be.

@djc
Copy link
Contributor

djc commented Mar 19, 2024

Maybe have a look at anstream (which is used by clap)?

https://docs.rs/anstream/latest/anstream/

IIRC it has a fairly aggressive MSRV policy which might not be a great fit.

@stefnotch
Copy link

@djc Thank you!

I looked into it just now, it's a lovely crate (more specifically, anstyle). It also doesn't seem to have that issue, since it's a more low level crate for working with terminal colors.

And it sadly isn't immediately usable, since it's MSRV is 1.70.0 🥲
https://github.com/rust-cli/anstyle/blob/476efbc8c68ae473eb17ea53385527a916226bbf/Cargo.toml#L8
while tracing-subscriber's MSRV is 1.63.0

rust-version = "1.63.0"

Definitely a lovely crate that I might remember for other projects.

@stefnotch stefnotch linked a pull request Mar 20, 2024 that will close this issue
@stefnotch
Copy link

anstream's MSRV has been lowered to 1.65.0. That's still higher than tracing-subscriber's MSRV though.
https://github.com/rust-cli/anstyle/blob/76f6849f826b3f73e827890e9abab5113820ffec/Cargo.toml#L8

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.

4 participants