Skip to content
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

proposal: os: File.IsTerminal #28572

Open
neild opened this Issue Nov 3, 2018 · 13 comments

Comments

Projects
None yet
6 participants
@neild
Copy link
Contributor

neild commented Nov 3, 2018

Add an IsTerminal method to os.File, equivalent to the Unix stdlib isatty():

// IsTerminal returns true if the file is associated with a terminal.
func (*File) IsTerminal() bool {}

The github.com/mattn/go-isatty package provides an equivalent function. It has 476 imports and 254 stars, which indicates that there is a fair degree of demand for this feature. I think the standard library should provide it.

@gopherbot gopherbot added this to the Proposal milestone Nov 3, 2018

@gopherbot gopherbot added the Proposal label Nov 3, 2018

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

ianlancetaylor commented Nov 3, 2018

At one time we discussed having a golang.org/x/terminal package to provide some more-or-less generic terminal handling capabilities.

I wonder if os.IsTerminal would be somewhat frustrating in the absence of anything portable you could do with a terminal, such as turning off echo. What would this be used for?

@neild

This comment has been minimized.

Copy link
Contributor Author

neild commented Nov 3, 2018

One thing I want it for is to let protoc-gen-go print an error if it's invoked interactively.

There's a progress bar library used fairly extensively inside Google which alters its output depending on whether it's writing to a terminal.

Looking through imports of the existing package would presumably find more uses.

A full terminal package would be nice, but I'd hate to make the perfect the enemy of the good. Plus, not all cases need a full terminal library.

@neild

This comment has been minimized.

Copy link
Contributor Author

neild commented Nov 3, 2018

As for what you can do with just this function: \r can get you a surprising way. Assuming the terminal supports the usual ANSI escape sequences if TERM is set and isatty(stderr) is fairly reasonable these days. (And points at why you might want IsTerminal, because ANSI escapes are more portable than a isatty implementation.)

I think it might be possible to write a full curses library using only existing functions in the os package--with the exception of this one.

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Nov 3, 2018

I agree that just knowing if a file is a terminal can be useful on its own. I currently use https://godoc.org/golang.org/x/crypto/ssh/terminal#IsTerminal, like:

terminal.IsTerminal(int(f.Fd()))

If this is simple enough to add and maintain, my vote is in favor of adding this to the standard library.

@tklauser

This comment has been minimized.

Copy link
Member

tklauser commented Nov 5, 2018

If this is simple enough to add and maintain, my vote is in favor of adding this to the standard library.

I think this should be simple enough to add by adding a copy of IoctlGetTermios from x/sys/unix to internal/syscall/unix and add/define the correct TCGET* value per GOOS/GOARCH for IoctlReadTermios. IsTerminal for all unix geese would then be:

import "internal/syscall/unix"

func (f *File) IsTerminal() bool {
        _, err := unix.IoctlGetTermios(f.Fd(), unix.IoctlReadTermios)
        return err == nil
} 

For windows everything that is needed is already in syscall, so IsTerminal would just be:

func (f *File) IsTerminal() bool {
        var st uint32
        err := syscall.GetConsoleMode(syscall.Handle(f.Fd()), &st)
        return err == nil
}  

I'm not sure about plan9. IsTerminal from x/crypto/ssh/terminal is currently:

func IsTerminal(fd int) bool {
        return false
}
@tklauser

This comment has been minimized.

Copy link
Member

tklauser commented Nov 6, 2018

I'm not sure about plan9. IsTerminal from x/crypto/ssh/terminal is currently:

func IsTerminal(fd int) bool {
        return false
}

Looking at https://plan9.io/sources/plan9/sys/src/ape/lib/ap/plan9/isatty.c, I think the following might work for plan9:

func (f *File) IsTerminal() bool {
        return strings.HasSuffix(f.name, "/dev/cons") 
}
@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Nov 28, 2018

One thing I want it for is to let protoc-gen-go print an error if it's invoked interactively.

Why not print the error always? The main objection I have to isatty is programs that change behavior based on whether they are run on a tty. If you think protoc-gen-go should be invoked with a pipe you could always check that (os.Stdin.Fstat).

@rsc rsc added the WaitingForInfo label Dec 12, 2018

@gopherbot

This comment has been minimized.

Copy link

gopherbot commented Jan 12, 2019

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

@gopherbot gopherbot closed this Jan 12, 2019

@neild

This comment has been minimized.

Copy link
Contributor Author

neild commented Jan 15, 2019

Why not print the error always? The main objection I have to isatty is programs that change behavior based on whether they are run on a tty. If you think protoc-gen-go should be invoked with a pipe you could always check that (os.Stdin.Fstat).

The error in this case is "don't run this interactively", so printing the error always isn't the right thing.

For the case of protoc-gen-go, it might be enough to stat os.Stdin and check for ModeNamedPipe (which I didn't realize is true for non-named pipes as well--very confusing).

Another common use of isatty where that would not, however, suffice is to determine whether to colorize or otherwise add additional formatting to output; in that case, you really do need to distinguish between a terminal and a file/pipe/any other destination.

@neild neild reopened this Jan 15, 2019

@gopherbot gopherbot closed this Jan 15, 2019

@mvdan mvdan removed the WaitingForInfo label Jan 15, 2019

@mvdan mvdan reopened this Jan 15, 2019

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Jan 15, 2019

@gopherbot tsk tsk.

@mvdan

This comment has been minimized.

Copy link
Member

mvdan commented Jan 15, 2019

While I generally agree with @rsc that programs shouldn't change their behavior depending on whether they're run interactively, I think developers will find a way to do that in any case. Worse even, they'll pull in x/sys/unix or mattn's small module just to get what would be a small method in the os package.

I think trying to discourage the use of isatty in most cases is understandable, but I don't think it's enough reason to keep it outside of the standard library. Granted that some developers will try to shoot themselves in the foot with an easily accessible IsTerminal, but there are plenty of other ways to do bad things with the standard library. We could always add a warning to the method's godoc.

One use case which I think is reasonable is whether or not to print a prompt in a shell-like program. For example:

$ dash <<EOF
> echo foo
> echo bar
> EOF
foo
bar
$ dash
$ echo foo
foo
$ echo bar
bar

I maintain such a program, and it's a shame that the only non-std dependency I pull is golang.org/x/crypto/ssh/terminal, just for this reason.

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Feb 6, 2019

We looked for a little while at maybe having a FileInfo mode bit, but the bit seems expensive to calculate. For this:

One thing I want it for is to let protoc-gen-go print an error if it's invoked interactively.

Would it suffice to stat os.Stdin and check for ModeCharDevice?

@neild

This comment has been minimized.

Copy link
Contributor Author

neild commented Feb 6, 2019

Would it suffice to stat os.Stdin and check for ModeCharDevice?

That's definitely not the same thing as isatty(), but maybe it suffices on Unix systems? Probably not anywhere else, though.

tklauser added a commit to tklauser/go that referenced this issue Mar 18, 2019

os: add File.IsTerminal
Just a proof of concept implementation for golang#28572, proposal needs to be
decided first.

DO NOT REVIEW
DO NOT SUBMIT

Change-Id: I360c6a01650629e938f9454cb5180f45c67b82b7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.