implemented IsTerminal() on Solaris #307
Conversation
| _, err := unix.IoctlGetTermios(int(os.Stdout.Fd()), unix.TCGETA) | ||
| fd := syscall.Stderr | ||
| var termio unix.Termio | ||
| err := unix.IoctlSetTermio(fd, unix.TCGETA, &termio) |
There was a problem hiding this comment.
won't this mutate the state of the file descriptor? seems like undesirable behavior from a method named IsTerminal
There was a problem hiding this comment.
I'm confused as to why this is calling IoctlSetTermio instead of IoctlGetTermio, and I'm also confused as to why this is assuming stderr specifically instead of stdout.
Can you explain that a bit more?
There was a problem hiding this comment.
@aybabtme & @binarycrusader: Good question. The code in question isn't present in the original git://golang.org/x/crypto/ssh/termina/util_solaris.go where I first implemented it.
stderr vs. stdout because I used terminal_notwindows.go as a starting point. Since IsTerminal() does not have an argument to specify which fd to test for terminal-ness, The rationale for @illicitonion's choice of stderr is here:
05f9567
I am no longer sure why I submitted this pull request. I suspect what happened is I implemented terminal_solaris.go some time ago to get InfluxDB or Prometheus to build on Solaris, and only later merged master back and pushed it, not realizing someone else had already implemented this independently:
8cdd4b3
I verified the code in the master branch works on Solaris (well, apart from the stdout vs. stderr issue). Feel free to disregard this pull request.
|
closed by 8cdd4b3 |
|
Releasing master which will work with Solaris. Thanks @binarycrusader and @fazalmajid |
This is a Solaris implementation of IsTerminal(), based on golang/go#13085