Skip to content

Please provide a portable isatty function for stdin/stdout/stderr #33736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
joshtriplett opened this issue May 19, 2016 · 17 comments
Closed

Please provide a portable isatty function for stdin/stdout/stderr #33736

joshtriplett opened this issue May 19, 2016 · 17 comments
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@joshtriplett
Copy link
Member

Some programs want to automatically emit color output, or spawn a pager, if output goes to a terminal, but not with redirected output. libtest has a function stdout_isatty, and rustc's libsyntax has a function stderr_isatty, with almost identical copy/pasted code other than the fd/handle checked. This strongly suggests the need for a library implementation.

Please consider providing a portable implementation of "isatty" for the Stdin, Stdout, and Stderr types, based on the above implementations from rustc/libtest.

@birkenfeld
Copy link
Contributor

There's an atty crate on crates.io. It doesn't seem to allow querying anything other than stdout at the moment.

@joshtriplett
Copy link
Member Author

joshtriplett commented May 19, 2016

@birkenfeld It also doesn't seem to work correctly on Windows: softprops/atty#3

I think it would make sense to add the code rustc is using to an appropriate library, to avoid duplicating it. Perhaps it would make sense to have a trait for this, and impl that trait for the Stdin, Stdout, and Stderr types?

(If this needs to go through an RFC rather than an issue filed against Rust, I can file one.)

@retep998
Copy link
Member

retep998 commented May 19, 2016

Right now libstd does not provide a way to get the HANDLE of stdout, so even if you could ask libstd whether it is a tty, you wouldn't be able to actually do any console shenanigans with it on Windows. If you call GetStdHandle to get stdout, now you're bypassing libstd's stdout, which means when output is thread locally redirected during testing, you'll bypass that redirection. Furthermore there isn't really a good way to tell whether a pipe is an msys/mintty pipe which can accept ansi escape codes, aside from getting the name of the pipe and guessing based on that.

@nagisa
Copy link
Member

nagisa commented May 19, 2016

For rustc we won’t have any problems with using external libraries (i.e. the atty crate) once we move to cargo-based build system. This is also somewhat against rust’s tendency to keep libstd small.

The atty crate not supporting windows is trivially fixed by porting the relevant code from rustc to there. Well, other than the HANDLE thing, which we should eventually expose.

@joshtriplett
Copy link
Member Author

@retep998

Right now libstd does not provide a way to get the HANDLE of stdout, so even if you could ask libstd whether it is a tty, you wouldn't be able to actually do any console shenanigans with it on Windows. If you call GetStdHandle to get stdout, now you're bypassing libstd's stdout, which means when output is thread locally redirected during testing, you'll bypass that redirection.

By "thread-local redirection", do you mean changing what std::io::stdout() refers to without changing the process's fd 1 (or Windows equivalent)? If Rust provides such a mechanism, then sure, supporting that seems useful.

Furthermore there isn't really a good way to tell whether a pipe is an msys/mintty pipe which can accept ansi escape codes, aside from calling an unstable NT function to get the name of the pipe and guessing based on that.

I don't think we want to return "true" for any kind of pipe. isatty just says if output is going to a terminal; if the caller wants to determine if they should send escape sequences to a pipe, that's a separate problem (e.g. --color=always, or a self-spawned pager known to support color like less -R).

@retep998
Copy link
Member

By "thread-local redirection", do you mean changing what std::io::stdout() refers to without changing the process's fd 1 (or Windows equivalent)? If Rust provides such a mechanism, then sure, supporting that seems useful.

In order to support libtest, which captures the output of tests so they don't get printed out, libstd does have its own internal thread local redirection. When you write to io::stdout() or call println! you are going through that abstraction. Also, because Rust caches the result of calling GetStdHandle, if something calls SetStdHandle then that will result in a disparity between io::stdout() and GetStdHandle as well.

I don't think we want to return "true" for any kind of pipe.

If you want to completely ignore all terminals that aren't the standard Windows console, sure. While I personally don't have a problem with throwing mintty under the bus, there are probably other people who do care.

Also note that since stdout and stderr can be redirected independently, you'd have to specify whether it is stdout or stderr to isatty. The atty crate doesn't provide an option for this.

@joshtriplett
Copy link
Member Author

If you want to completely ignore all terminals that aren't the standard Windows console, sure. While I personally don't have a problem with throwing mintty under the bus, there are probably other people who do care.

Short of programs providing an option to override the detection (e.g. --color=always or an equivalent via environment or config file), I don't see any obvious way around that. If the fd or HANDLE looks like a pipe, how can the program tell the difference between a pipe to a terminal that handles escape sequences, and a pipe to a program that doesn't?

@dtolnay
Copy link
Member

dtolnay commented Jun 6, 2016

In the meantime, I factored Cargo's implementation into the isatty crate because I needed it for cargo-expand.

@joshtriplett
Copy link
Member Author

joshtriplett commented Jun 6, 2016

@dtolnay Thanks!

@joshtriplett
Copy link
Member Author

@retep998 Do you have a pointer to the thread-local redirection mechanism in libstd? I can't seem to find anything about it, except some hints that such a mechanism existed in old_io.

@joshtriplett
Copy link
Member Author

@retep998 Thanks. Unstable, with documentation hidden; no wonder I didn't find it.

@steveklabnik steveklabnik added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed A-libs labels Mar 24, 2017
@Mark-Simulacrum
Copy link
Member

The https://github.com/softprops/atty crate now also does this. I'm somewhat uncertain that we should implement such a function in libstd, but wouldn't be against it either.

@Mark-Simulacrum Mark-Simulacrum added the C-feature-request Category: A feature request, i.e: not implemented / a PR. label Jul 25, 2017
@dtolnay
Copy link
Member

dtolnay commented Nov 16, 2017

I agree that isatty information seems appropriate to provide for the standard library Stdin Stdout Stderr types. I would be interested in seeing a concrete design proposed in an RFC if someone is interested in spearheading that.

@dtolnay dtolnay closed this as completed Nov 16, 2017
@retep998
Copy link
Member

Unless libstd wants to tackle the task of providing TTY specific functionality such as colors and stuff using ANSI escape codes and windows console api calls, then there is very little reason for libstd to provide isatty.

@joshtriplett
Copy link
Member Author

If someone would be interested in proposing an RFC, I'd be happy to mentor them through the process.

@joshtriplett
Copy link
Member Author

@retep998 There are other reasons to want to know if stdout or stderr is a tty, besides formatting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

8 participants