rust-cli / env_logger

A logging implementation for `log` which is configured via an environment variable.
https://docs.rs/env_logger
Apache License 2.0
782 stars 124 forks source link

Replace is-terminal dep with std::io::IsTerminal #276

Closed LunNova closed 5 months ago

LunNova commented 10 months ago

https://doc.rust-lang.org/stable/std/io/trait.IsTerminal.html

Needs to wait for MSRV 1.70

epage commented 10 months ago

Looks like we don't have a state MSRV policy. My default choice is N-2 but I suspect that env_logger's wider use might make that unappealing.

@matthiasbeyer any thoughts?

matthiasbeyer commented 10 months ago

Yeah so I would actually keep this crate as backwards compatible as possible within reason. If we currently compile on N-10 or even older, I don't see why we should change that if we do not have some really good reasons.

Rationale is that this is, IMO, the go-to crate for "I want logging". It's the lowest-hanging fruit for getting logging to work in a Rust project, at least in my perception.

I would like to see this issue resolved, but I think backwards compatibility is more important.

I'm open to discussion though.

LunNova commented 10 months ago

Might be possible to use cfg(not(version("1.70"))) to use that dep only on old versions and avoid bumping MSRV. Haven't tried, not sure if you can do that in Cargo.toml. definitely not possible, cfg(version) isn't stable

epage commented 8 months ago

Another change I'm looking to make, switching from termcolor to anstream, would require an MSRV bump and would lead to a breaking change (removal of the env_logger styling API).

While I'm not a fan of treating MSRV as a breaking change, if its done in conjunction with breakings changes (with a commitment to support old versions if needed), I wonder if that would be sufficient to allow us to move forward with this.

kayabaNerve commented 7 months ago

You can write a build script such that it sets a feature for either is-terminal or std-io-isterminal based on the version in use. That wouldn't be breaking.

emilk commented 6 months ago

I'd love to see this too. Getting rid of the is-terminal dependency tree would be lovely:

is-terminal v0.4.7
├── io-lifetimes v1.0.10
│   └── libc v0.2.150
└── rustix v0.37.19
    ├── bitflags v1.3.2
    ├── errno v0.3.1
    │   └── libc v0.2.150
    ├── io-lifetimes v1.0.10 (*)
    └── libc v0.2.150
epage commented 5 months ago

I've gone ahead with my idea of bundling the MSRV bump with a pre-planned breaking change of switching from termcolor to anstream. Longer term, I'm hoping rust-lang/rfcs#3537 will help reduce the conflict between the users who want the latest and those who need an earlier version.

See #298