Fixup for #1598.

- Let file:is-tty always take one argument.

- Revert change to eval.ByteOutput.

- Make sys.IsATTY take a FD instead, to avoid the need to use os.NewFile. Using
  os.NewFile can cause the Go runtime to start polling the file, which
  interferes with Elvish's terminal reader.

pkg/eval/port.go

@@ -294,17 +294,12 @@ func (vo valueOutput) Put(v any) error {
 type ByteOutput interface {
 	io.Writer
 	io.StringWriter
-	File() *os.File
 }
 
 type byteOutput struct {
 	f *os.File
 }
 
-func (bo byteOutput) File() *os.File {
-	return bo.f
-}
-
 func (bo byteOutput) Write(p []byte) (int, error) {
 	n, err := bo.f.Write(p)
 	return n, convertReaderGone(err)

pkg/eval/process_unix.go

@@ -14,7 +14,7 @@ import (
 // Process control functions in Unix.
 
 func putSelfInFg() error {
-	if !sys.IsATTY(os.Stdin) {
+	if !sys.IsATTY(os.Stdin.Fd()) {
 		return nil
 	}
 	// If Elvish is in the background, the tcsetpgrp call below will either fail

pkg/mods/file/file.go

@@ -8,13 +8,14 @@ import (
 	"src.elv.sh/pkg/eval"
 	"src.elv.sh/pkg/eval/errs"
 	"src.elv.sh/pkg/eval/vals"
+	"src.elv.sh/pkg/parse"
 	"src.elv.sh/pkg/sys"
 )
 
 var Ns = eval.BuildNsNamed("file").
 	AddGoFns(map[string]any{
 		"close":    close,
-		"is-tty":   isTty,
+		"is-tty":   isTTY,
 		"open":     open,
 		"pipe":     pipe,
 		"truncate": truncate,
@@ -23,46 +24,54 @@ var Ns = eval.BuildNsNamed("file").
 //elvdoc:fn is-tty
 //
 // ```elvish
-// file:is-tty $file-obj?
+// file:is-tty $file
 // ```
 //
-// Outputs `$true` if `$file-obj` is open on a tty (i.e., a terminal device);
-// otherwise, outputs `$false`. If the `$file-obj` argument is omitted the
-// default output byte stream is tested.
+// Outputs whether `$file` is a terminal device.
+//
+// The `$file` can be a file object or a number. If it's a number, it's
+// interpreted as a numerical file descriptor.
 //
 // ```elvish-transcript
-// ~> file:is-tty
-// ▶ $true
-// ~> var fh = (file:open /dev/tty)
-// ~> file:is-tty $fh
+// ~> var f = (file:open /dev/tty)
+// ~> file:is-tty $f
 // ▶ $true
-// ~> if (file:is-tty) { echo no } else { echo yes }
-// yes
-// ~> var fh = (file:pipe)
-// ~> file:is-tty $fh
+// ~> file:close $f
+// ~> var f = (file:open /dev/null)
+// ~> file:is-tty $f
 // ▶ $false
-// ~> var fh = (file:open /dev/null)
-// ~> file:is-tty $fh
+// ~> file:close $f
+// ~> var p = (file:pipe)
+// ~> file:is-tty $p[r]
 // ▶ $false
+// ~> file:is-tty $p[w]
+// ▶ $false
+// ~> file:close $p[r]
+// ~> file:close $p[w]
+// ~> file:is-tty 0
+// ▶ $true
+// ~> file:is-tty 1
+// ▶ $true
+// ~> file:is-tty 2
+// ▶ $true
 // ```
 
-func isTty(fm *eval.Frame, fileObj ...any) (bool, error) {
-	switch len(fileObj) {
-	case 0:
-		if sys.IsATTY(fm.ByteOutput().File()) {
-			return true, nil
-		}
-		return false, nil
-	case 1:
-		if f, ok := fileObj[0].(*os.File); ok {
-			if sys.IsATTY(f) {
-				return true, nil
-			}
+func isTTY(fm *eval.Frame, file any) (bool, error) {
+	switch file := file.(type) {
+	case *os.File:
+		return sys.IsATTY(file.Fd()), nil
+	case int:
+		return sys.IsATTY(uintptr(file)), nil
+	case string:
+		var fd int
+		if err := vals.ScanToGo(file, &fd); err != nil {
+			return false, errs.BadValue{What: "argument to file:is-tty",
+				Valid: "file value or numerical FD", Actual: parse.Quote(file)}
 		}
-		return false, nil
+		return sys.IsATTY(uintptr(fd)), nil
 	default:
-		return false, errs.ArityMismatch{
-			What: "arguments", ValidLow: 0, ValidHigh: 1, Actual: len(fileObj)}
+		return false, errs.BadValue{What: "argument to file:is-tty",
+			Valid: "file value or numerical FD", Actual: vals.ToString(file)}
 	}
 }
 

pkg/mods/file/file_test.go

@@ -69,11 +69,20 @@ func TestFile(t *testing.T) {
 			Valid: "integer", Actual: "non-integer",
 		}),
 
-		// TODO: Improve these tests if, and when, https://b.elv.sh/1595 is resolved.
-		That("file:is-tty").Puts(false),
-		That("file:is-tty x").Puts(false),
-		That("var p = (file:pipe); file:is-tty $p[w]").Puts(false),
-		That("file:is-tty x y").Throws(ErrorWithType(errs.ArityMismatch{})),
+		// TODO: Test with PTY when https://b.elv.sh/1595 is resolved.
+		That("file:is-tty 0").Puts(false),
+		That("file:is-tty (num 0)").Puts(false),
+		That(
+			"var p = (file:pipe)",
+			"file:is-tty $p[r]; file:is-tty $p[w]",
+			"file:close $p[r]; file:close $p[w]").
+			Puts(false, false),
+		That("file:is-tty a").
+			Throws(errs.BadValue{What: "argument to file:is-tty",
+				Valid: "file value or numerical FD", Actual: "a"}),
+		That("file:is-tty []").
+			Throws(errs.BadValue{What: "argument to file:is-tty",
+				Valid: "file value or numerical FD", Actual: "[]"}),
 	)
 
 	fi, err := os.Stat("file100")

pkg/shell/interact.go

@@ -70,7 +70,7 @@ func interact(ev *eval.Evaler, fds [3]*os.File, cfg *interactCfg) {
 
 	// Build Editor.
 	var ed editor
-	if sys.IsATTY(fds[0]) {
+	if sys.IsATTY(fds[0].Fd()) {
 		newed := edit.NewEditor(cli.NewTTY(fds[0], fds[2]), ev, daemonClient)
 		ev.ExtendBuiltin(eval.BuildNs().AddNs("edit", newed))
 		ev.BgJobNotify = func(s string) { newed.Notify(ui.T(s)) }

pkg/sys/sys.go

@@ -21,6 +21,6 @@ const SIGWINCH = sigWINCH
 func WinSize(file *os.File) (row, col int) { return winSize(file) }
 
 // IsATTY determines whether the given file is a terminal.
-func IsATTY(file *os.File) bool {
-	return isatty.IsTerminal(file.Fd()) || isatty.IsCygwinTerminal(file.Fd())
+func IsATTY(fd uintptr) bool {
+	return isatty.IsTerminal(fd) || isatty.IsCygwinTerminal(fd)
 }