posixutils-rs icon indicating copy to clipboard operation
posixutils-rs copied to clipboard

sh: remove unnecessary atty dep

Open joshka opened this issue 8 months ago • 3 comments

IsTerminal trait was introduced in Rust 1.70

joshka avatar Apr 03 '25 00:04 joshka

Thanks for the patch.

Quick review: ok, but should be applied across entire tree. We don't want to avoid atty in one util, yet use atty in another util.

jgarzik avatar Apr 03 '25 03:04 jgarzik

Quick review: ok, but should be applied across entire tree. We don't want to avoid atty in one util, yet use atty in another util.

This was the only instance of the atty crate in the main branch (are the other branches active development?). The atty crate is no longer in the cargo.lock at all: https://github.com/rustcoreutils/posixutils-rs/blob/fa17a3118e4bfbf3c19e7f04599e8fa80e5d288a/Cargo.lock

There are some calls to use libc::isatty not included in this change (will make another PR), and mention of atty in the contributing doc (which I've removed in https://github.com/rustcoreutils/posixutils-rs/commit/594aa1e0e8f26e1844245d9b6bc69b5d84af581f).

joshka avatar Apr 03 '25 04:04 joshka

Actually - the replacement for the libc::isatty call was relatively simple.

https://github.com/rustcoreutils/posixutils-rs/pull/399/commits/76fa884a135af72ef138f6511d6391dcbb639df8 https://github.com/rustcoreutils/posixutils-rs/pull/399/commits/cf26e81a49b764f2f532285907d3223b62fa4b8f - removed libc from the deps for test

Manually tested this with:

target/debug/test -t 0 || echo foo
echo foo target/debug/test -t 0 || echo foo # no output
target/debug/test -t 1 || echo foo
target/debug/test -t 1 1&>/dev/null || echo foo # no output
target/debug/test -t 2 || echo foo
target/debug/test -t 2 2&>/dev/null || echo foo # no output
target/debug/test -t 3 || echo foo # no output
target/debug/test -t 3 3&>/dev/tty || echo foo
target/debug/test -t asdf || echo foo # no output

joshka avatar Apr 03 '25 04:04 joshka

thanks - looks like rust std isTerminal is the best approach

jgarzik avatar Dec 01 '25 00:12 jgarzik

I'm not sure I follow. Using IsTerminal is what this PR does.

joshka avatar Dec 01 '25 03:12 joshka