dalance / termbg

A Rust library for terminal background color detection
Apache License 2.0
87 stars 8 forks source link

API without using stdin #6

Open dalance opened 3 years ago

dalance commented 3 years ago

https://github.com/zellij-org/zellij/issues/538#issuecomment-850369801

tavianator commented 2 years ago

It's wrong to assume that stdin is connected to the same terminal as stdout/stderr. For a basic example,

tavianator@tachyon $ cargo run </dev/null
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s
     Running `target/debug/termbg`
Check terminal background color
  Term : XtermCompatible
  Latency: 2.855647ms
  Color: detection failed Io { source: Kind(UnexpectedEof) }
  Theme: detection failed Io { source: Kind(UnexpectedEof) }
tavianator@tachyon $ n11;rgb:0000/0000/000011;rgb:0000/0000/0000

Instead, I think the right thing to do is to open the same device as stderr, but for reading.

Crossterm does this, which might be okay too: https://github.com/crossterm-rs/crossterm/blob/58f580eaad4e80ccb8a09541b760a329971bb4bc/src/event/sys/unix/file_descriptor.rs#L66-L82

joshtriplett commented 2 years ago

I don't think it's safe to assume that the stderr device supports reading. For instance, consider redirecting stderr but keeping stdout/stdin.

I think it's a reasonable default assumption to use stdin and stdout. But it does make sense to offer the option of passing separate Read and Write instances.

tavianator commented 2 years ago

I don't think it's safe to assume that the stderr device supports reading. For instance, consider redirecting stderr but keeping stdout/stdin.

RIght, but you can check whether stderr is connected to a TTY with isatty(), and if it's not already open for reading you can find the device with ttyname() and reopen it with the right permissions. I prototyped that here: https://gist.github.com/tavianator/d66d425399a57c51629999ae716bbd24#file-lib-rs-L143

joshtriplett commented 2 years ago

Good point. That does seem reasonable: whatever device you're trying to figure out the properties of is the one you can check like that.

Also, for future use: https://github.com/rust-lang/rust/issues/98070