Conversation
Switch is-terminal on Unix and WASI to use `libc` instead of `rustix` by default. This matches the `IsTerminal` implementation in std, and avoids pulling in all of rustix as a dependency just for one function. This retains rustix as an option, which may be enabled by using `default-features = false` and enabling either the `rustix-std` or `rustix-no_std` features.
|
This likely fails to compile if you enable both rustix and libc. You likely wouldn't do this directly, but two of your dependencies might enable one feature each, leaving you with a project that can't compile. What is the intent with keeping rustix as a dependency? What is being gained? I'm failing to understand why one might prefer this approach to the one in my PR. |
|
Features indeed need to be additive only, and not internally incompatible with each other. If a solution like this is merged, it has to be able to pick one way of doing it in case both are enabled, and still compile and work. |
|
|
||
| [features] | ||
| default = ["std", "libc"] | ||
| std = [] |
There was a problem hiding this comment.
What does this feature do? Just enabling it by itself does not pull anything in.
If the features should represent three states this crate can be in:
- No rustix (use libc)
- Rustix but no_std
- Rustix with std.
... Then I would consider defining the features like this:
[features]
default = []
# Implicit `rustix` feature by the optional dependency.
# Switches from `libc` to `rustix` backend.
# Enabling `std` makes is-terminal use the `rustix` backend, with std support.
std = ["rustix", "rustix/std"]
The combination libc + std does not seem to do anything differently compared to just libc from what I can see in the current PR code.
But I have to say I'm shooting a bit from the hip here. I don't know why this crate needs two backends. Out of curiosity, why is rustix wanted? Does it yield more accurate results compared to libc in some situations?
| not(any(feature = "rustix", feature = "libc")) | ||
| ))] | ||
| { | ||
| compile_error!("either \"rustix\" or \"libc\" must be enabled"); |
There was a problem hiding this comment.
does libc really need to be optional? rustix has libc as a dependency anyway, so by opting for the rustix backend you don't get rid of libc from the dependency tree anyway. If you make libc non-optional and it uses it by default unless a rustix feature is enabled, there won't be any combination of features yielding a compiler error.
|
Instead of this PR, I've now created a separate rustix-is-terminal crate as a way to support my use cases that want to continue using the rustix-based implementation, so that we can proceed with #26. |
Switch is-terminal on Unix and WASI to use
libcinstead ofrustixby default. This matches theIsTerminalimplementation in std, and avoids pulling in all of rustix as a dependency just for one function.This retains rustix as an option, which may be enabled by using
default-features = falseand enabling either therustix-stdorrustix-no_stdfeatures.